commit 93cf913073d94f6e00bdf95196067cfd996daeef
author Linus Nilsson <lnilsson@google.com> Thu Sep 24 12:12:48 2020 -0700
committer Linus Nilsson <lnilsson@google.com> Fri Sep 25 16:15:55 2020 -0700
tree 5fb2c3e3038a8eb9bb91f10298aeb5dcdcfca39c
parent c31d2491af85218c29845840c46ff3fa5df6b1bc

Transcoder: Fixed codec leaks on cancel.

When transcoding was cancelled, the VideoTrackTranscoder
held strong references to itself through its internal
codec handler work queue.
This commit clears the work queue and blocks new messages
from being stored on cancel.

Test: Unit test and address sanitizer.
Fixes: 168522198
Change-Id: I9c3a4469378a91a492c6872bafc6d494dadf2853

media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp

diff --git a/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp b/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp
index 53bd2a2..7a968eb 100644
--- a/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp
+++ b/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp
@@ -20,6 +20,7 @@
 #define LOG_TAG "MediaTranscoderTests"
 
 #include <android-base/logging.h>
+#include <android/binder_process.h>
 #include <fcntl.h>
 #include <gtest/gtest.h>
 #include <media/MediaSampleReaderNDK.h>
@@ -72,7 +73,13 @@
     }
 
     virtual void onProgressUpdate(const MediaTranscoder* transcoder __unused,
-                                  int32_t progress __unused) override {}
+                                  int32_t progress) override {
+        std::unique_lock<std::mutex> lock(mMutex);
+        if (progress > 0 && !mProgressMade) {
+            mProgressMade = true;
+            mCondition.notify_all();
+        }
+    }
 
     virtual void onCodecResourceLost(const MediaTranscoder* transcoder __unused,
                                      const std::shared_ptr<const Parcel>& pausedState
@@ -85,12 +92,19 @@
         }
     }
 
+    void waitForProgressMade() {
+        std::unique_lock<std::mutex> lock(mMutex);
+        while (!mProgressMade && !mFinished) {
+            mCondition.wait(lock);
+        }
+    }
     media_status_t mStatus = AMEDIA_OK;
 
 private:
     std::mutex mMutex;
     std::condition_variable mCondition;
     bool mFinished = false;
+    bool mProgressMade = false;
 };
 
 // Write-only, create file if non-existent, don't overwrite existing file.
@@ -106,6 +120,7 @@
     void SetUp() override {
         LOG(DEBUG) << "MediaTranscoderTests set up";
         mCallbacks = std::make_shared<TestCallbacks>();
+        ABinderProcess_startThreadPool();
     }
 
     void TearDown() override {
@@ -126,9 +141,16 @@
         return (float)diff * 100.0f / s1.st_size;
     }
 
+    typedef enum {
+        kRunToCompletion,
+        kCancelAfterProgress,
+        kCancelAfterStart,
+    } TranscodeExecutionControl;
+
     using FormatConfigurationCallback = std::function<AMediaFormat*(AMediaFormat*)>;
     media_status_t transcodeHelper(const char* srcPath, const char* destPath,
-                                   FormatConfigurationCallback formatCallback) {
+                                   FormatConfigurationCallback formatCallback,
+                                   TranscodeExecutionControl executionControl = kRunToCompletion) {
         auto transcoder = MediaTranscoder::create(mCallbacks, nullptr);
         EXPECT_NE(transcoder, nullptr);
 
@@ -160,7 +182,18 @@
         media_status_t startStatus = transcoder->start();
         EXPECT_EQ(startStatus, AMEDIA_OK);
         if (startStatus == AMEDIA_OK) {
-            mCallbacks->waitForTranscodingFinished();
+            switch (executionControl) {
+            case kCancelAfterProgress:
+                mCallbacks->waitForProgressMade();
+                FALLTHROUGH_INTENDED;
+            case kCancelAfterStart:
+                transcoder->cancel();
+                break;
+            case kRunToCompletion:
+            default:
+                mCallbacks->waitForTranscodingFinished();
+                break;
+            }
         }
         close(srcFd);
         close(dstFd);
@@ -310,6 +343,41 @@
     EXPECT_GT(getFileSizeDiffPercent(destPath1, destPath2), 40);
 }
 
+static AMediaFormat* getAVCVideoFormat(AMediaFormat* sourceFormat) {
+    AMediaFormat* format = nullptr;
+    const char* mime = nullptr;
+    AMediaFormat_getString(sourceFormat, AMEDIAFORMAT_KEY_MIME, &mime);
+
+    if (strncmp(mime, "video/", 6) == 0) {
+        format = AMediaFormat_new();
+        AMediaFormat_setString(format, AMEDIAFORMAT_KEY_MIME, AMEDIA_MIMETYPE_VIDEO_AVC);
+    }
+
+    return format;
+}
+
+TEST_F(MediaTranscoderTests, TestCancelAfterProgress) {
+    const char* srcPath = "/data/local/tmp/TranscodingTestAssets/longtest_15s.mp4";
+    const char* destPath = "/data/local/tmp/MediaTranscoder_Cancel.MP4";
+
+    for (int i = 0; i < 32; ++i) {
+        EXPECT_EQ(transcodeHelper(srcPath, destPath, getAVCVideoFormat, kCancelAfterProgress),
+                  AMEDIA_OK);
+        mCallbacks = std::make_shared<TestCallbacks>();
+    }
+}
+
+TEST_F(MediaTranscoderTests, TestCancelAfterStart) {
+    const char* srcPath = "/data/local/tmp/TranscodingTestAssets/longtest_15s.mp4";
+    const char* destPath = "/data/local/tmp/MediaTranscoder_Cancel.MP4";
+
+    for (int i = 0; i < 32; ++i) {
+        EXPECT_EQ(transcodeHelper(srcPath, destPath, getAVCVideoFormat, kCancelAfterStart),
+                  AMEDIA_OK);
+        mCallbacks = std::make_shared<TestCallbacks>();
+    }
+}
+
 }  // namespace android
 
 int main(int argc, char** argv) {