Merge "Transcoder: Fixed codec leaks on cancel."
diff --git a/media/libmediatranscoding/transcoder/Android.bp b/media/libmediatranscoding/transcoder/Android.bp
index c153a42..258ed9a 100644
--- a/media/libmediatranscoding/transcoder/Android.bp
+++ b/media/libmediatranscoding/transcoder/Android.bp
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-cc_library_shared {
- name: "libmediatranscoder",
+cc_defaults {
+ name: "mediatranscoder_defaults",
srcs: [
"MediaSampleQueue.cpp",
@@ -60,3 +60,17 @@
cfi: true,
},
}
+
+cc_library_shared {
+ name: "libmediatranscoder",
+ defaults: ["mediatranscoder_defaults"],
+}
+
+cc_library_shared {
+ name: "libmediatranscoder_asan",
+ defaults: ["mediatranscoder_defaults"],
+
+ sanitize: {
+ address: true,
+ },
+}
diff --git a/media/libmediatranscoding/transcoder/MediaTranscoder.cpp b/media/libmediatranscoding/transcoder/MediaTranscoder.cpp
index 35bdc40..f35ea99 100644
--- a/media/libmediatranscoding/transcoder/MediaTranscoder.cpp
+++ b/media/libmediatranscoding/transcoder/MediaTranscoder.cpp
@@ -155,7 +155,7 @@
}
void MediaTranscoder::onTrackError(const MediaTrackTranscoder* transcoder, media_status_t status) {
- LOG(DEBUG) << "TrackTranscoder " << transcoder << " returned error " << status;
+ LOG(ERROR) << "TrackTranscoder " << transcoder << " returned error " << status;
sendCallback(status);
}
diff --git a/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp b/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp
index c7d775c..5579868 100644
--- a/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp
+++ b/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp
@@ -40,7 +40,11 @@
template <typename T>
void VideoTrackTranscoder::BlockingQueue<T>::push(T const& value, bool front) {
{
- std::unique_lock<std::mutex> lock(mMutex);
+ std::scoped_lock lock(mMutex);
+ if (mAborted) {
+ return;
+ }
+
if (front) {
mQueue.push_front(value);
} else {
@@ -52,7 +56,7 @@
template <typename T>
T VideoTrackTranscoder::BlockingQueue<T>::pop() {
- std::unique_lock<std::mutex> lock(mMutex);
+ std::unique_lock lock(mMutex);
while (mQueue.empty()) {
mCondition.wait(lock);
}
@@ -61,6 +65,14 @@
return value;
}
+// Note: Do not call if another thread might waiting in pop.
+template <typename T>
+void VideoTrackTranscoder::BlockingQueue<T>::abort() {
+ std::scoped_lock lock(mMutex);
+ mAborted = true;
+ mQueue.clear();
+}
+
// The CodecWrapper class is used to let AMediaCodec instances outlive the transcoder object itself
// by giving the codec a weak pointer to the transcoder. Codecs wrapped in this object are kept
// alive by the transcoder and the codec's outstanding buffers. Once the transcoder stops and all
@@ -484,12 +496,14 @@
message();
}
+ mCodecMessageQueue.abort();
+ AMediaCodec_stop(mDecoder);
+
// Return error if transcoding was stopped before it finished.
if (mStopRequested && !mEosFromEncoder && mStatus == AMEDIA_OK) {
mStatus = AMEDIA_ERROR_UNKNOWN; // TODO: Define custom error codes?
}
- AMediaCodec_stop(mDecoder);
return mStatus;
}
diff --git a/media/libmediatranscoding/transcoder/include/media/VideoTrackTranscoder.h b/media/libmediatranscoding/transcoder/include/media/VideoTrackTranscoder.h
index 0a7bf33..d000d7f 100644
--- a/media/libmediatranscoding/transcoder/include/media/VideoTrackTranscoder.h
+++ b/media/libmediatranscoding/transcoder/include/media/VideoTrackTranscoder.h
@@ -51,11 +51,13 @@
public:
void push(T const& value, bool front = false);
T pop();
+ void abort();
private:
std::mutex mMutex;
std::condition_variable mCondition;
std::deque<T> mQueue;
+ bool mAborted = false;
};
class CodecWrapper;
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