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
diff --git a/media/libmediatranscoding/transcoder/tests/Android.bp b/media/libmediatranscoding/transcoder/tests/Android.bp
index 4160c30..7ae6261 100644
--- a/media/libmediatranscoding/transcoder/tests/Android.bp
+++ b/media/libmediatranscoding/transcoder/tests/Android.bp
@@ -17,7 +17,7 @@
"libbase",
"libcutils",
"libmediandk",
- "libmediatranscoder",
+ "libmediatranscoder_asan",
"libutils",
],
@@ -26,6 +26,15 @@
"-Wall",
],
+ sanitize: {
+ misc_undefined: [
+ "unsigned-integer-overflow",
+ "signed-integer-overflow",
+ ],
+ cfi: true,
+ address: true,
+ },
+
data: [":test_assets"],
test_config_template: "AndroidTestTemplate.xml",
test_suites: ["device-tests", "TranscoderTests"],
@@ -80,4 +89,5 @@
name: "MediaTranscoderTests",
defaults: ["testdefaults"],
srcs: ["MediaTranscoderTests.cpp"],
+ shared_libs: ["libbinder_ndk"],
}
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) {
diff --git a/media/libmediatranscoding/transcoder/tests/build_and_run_all_unit_tests.sh b/media/libmediatranscoding/transcoder/tests/build_and_run_all_unit_tests.sh
index 241178d..b848b4c 100755
--- a/media/libmediatranscoding/transcoder/tests/build_and_run_all_unit_tests.sh
+++ b/media/libmediatranscoding/transcoder/tests/build_and_run_all_unit_tests.sh
@@ -25,22 +25,22 @@
echo "========================================"
echo "testing MediaSampleReaderNDK"
-adb shell /data/nativetest64/MediaSampleReaderNDKTests/MediaSampleReaderNDKTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/MediaSampleReaderNDKTests/MediaSampleReaderNDKTests
echo "testing MediaSampleQueue"
-adb shell /data/nativetest64/MediaSampleQueueTests/MediaSampleQueueTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/MediaSampleQueueTests/MediaSampleQueueTests
echo "testing MediaTrackTranscoder"
-adb shell /data/nativetest64/MediaTrackTranscoderTests/MediaTrackTranscoderTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/MediaTrackTranscoderTests/MediaTrackTranscoderTests
echo "testing VideoTrackTranscoder"
-adb shell /data/nativetest64/VideoTrackTranscoderTests/VideoTrackTranscoderTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/VideoTrackTranscoderTests/VideoTrackTranscoderTests
echo "testing PassthroughTrackTranscoder"
-adb shell /data/nativetest64/PassthroughTrackTranscoderTests/PassthroughTrackTranscoderTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/PassthroughTrackTranscoderTests/PassthroughTrackTranscoderTests
echo "testing MediaSampleWriter"
-adb shell /data/nativetest64/MediaSampleWriterTests/MediaSampleWriterTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/MediaSampleWriterTests/MediaSampleWriterTests
echo "testing MediaTranscoder"
-adb shell /data/nativetest64/MediaTranscoderTests/MediaTranscoderTests
+adb shell ASAN_OPTIONS=detect_container_overflow=0 /data/nativetest64/MediaTranscoderTests/MediaTranscoderTests