Transcoder: Fix video track transcoding crash.
The VideoTrackTranscoder had a bug where the encoder could
outlive the transcoder object, because the encoder owns the
output buffers. This caused a crash when the encoder sent async
callbacks to the transcoder after it had been released. This fix
gives the encoder a weak reference to the transcoder and gives
outstanding buffers a strong reference to the encoder.
Fixes: 160711746
Test: Transcoder unit tests (build_and_run_all_unit_tests.sh).
Test: New unit test to trigger the crash before the fix.
Change-Id: I8141591399b6cff642d2d322809d3254adbefaaf
diff --git a/media/libmediatranscoding/transcoder/tests/MediaTrackTranscoderTests.cpp b/media/libmediatranscoding/transcoder/tests/MediaTrackTranscoderTests.cpp
index 71d3a4e..502d5aa 100644
--- a/media/libmediatranscoding/transcoder/tests/MediaTrackTranscoderTests.cpp
+++ b/media/libmediatranscoding/transcoder/tests/MediaTrackTranscoderTests.cpp
@@ -53,7 +53,7 @@
switch (GetParam()) {
case VIDEO:
- mTranscoder = std::make_shared<VideoTrackTranscoder>(mCallback);
+ mTranscoder = VideoTrackTranscoder::create(mCallback);
break;
case PASSTHROUGH:
mTranscoder = std::make_shared<PassthroughTrackTranscoder>(mCallback);
@@ -164,8 +164,8 @@
ASSERT_TRUE(mTranscoder->start());
drainOutputSampleQueue();
EXPECT_EQ(mCallback->waitUntilFinished(), AMEDIA_OK);
- EXPECT_TRUE(mTranscoder->stop());
joinDrainThread();
+ EXPECT_TRUE(mTranscoder->stop());
EXPECT_FALSE(mQueueWasAborted);
EXPECT_TRUE(mGotEndOfStream);
}
@@ -232,9 +232,9 @@
ASSERT_TRUE(mTranscoder->start());
drainOutputSampleQueue();
EXPECT_EQ(mCallback->waitUntilFinished(), AMEDIA_OK);
+ joinDrainThread();
EXPECT_TRUE(mTranscoder->stop());
EXPECT_FALSE(mTranscoder->start());
- joinDrainThread();
EXPECT_FALSE(mQueueWasAborted);
EXPECT_TRUE(mGotEndOfStream);
}
@@ -247,9 +247,8 @@
mTranscoderOutputQueue->abort();
drainOutputSampleQueue();
EXPECT_EQ(mCallback->waitUntilFinished(), AMEDIA_ERROR_IO);
- EXPECT_TRUE(mTranscoder->stop());
-
joinDrainThread();
+ EXPECT_TRUE(mTranscoder->stop());
EXPECT_TRUE(mQueueWasAborted);
EXPECT_FALSE(mGotEndOfStream);
}
@@ -265,8 +264,8 @@
drainOutputSampleQueue();
EXPECT_EQ(mCallback->waitUntilFinished(), AMEDIA_OK);
- EXPECT_TRUE(mTranscoder->stop());
joinDrainThread();
+ EXPECT_TRUE(mTranscoder->stop());
EXPECT_FALSE(mQueueWasAborted);
EXPECT_TRUE(mGotEndOfStream);
diff --git a/media/libmediatranscoding/transcoder/tests/VideoTrackTranscoderTests.cpp b/media/libmediatranscoding/transcoder/tests/VideoTrackTranscoderTests.cpp
index 1eb9e5a..5f2cd12 100644
--- a/media/libmediatranscoding/transcoder/tests/VideoTrackTranscoderTests.cpp
+++ b/media/libmediatranscoding/transcoder/tests/VideoTrackTranscoderTests.cpp
@@ -95,12 +95,13 @@
TEST_F(VideoTrackTranscoderTests, SampleSanity) {
LOG(DEBUG) << "Testing SampleSanity";
std::shared_ptr<TestCallback> callback = std::make_shared<TestCallback>();
- VideoTrackTranscoder transcoder{callback};
+ auto transcoder = VideoTrackTranscoder::create(callback);
- EXPECT_EQ(transcoder.configure(mMediaSampleReader, mTrackIndex, mDestinationFormat), AMEDIA_OK);
- ASSERT_TRUE(transcoder.start());
+ EXPECT_EQ(transcoder->configure(mMediaSampleReader, mTrackIndex, mDestinationFormat),
+ AMEDIA_OK);
+ ASSERT_TRUE(transcoder->start());
- std::shared_ptr<MediaSampleQueue> outputQueue = transcoder.getOutputQueue();
+ std::shared_ptr<MediaSampleQueue> outputQueue = transcoder->getOutputQueue();
std::thread sampleConsumerThread{[&outputQueue] {
uint64_t sampleCount = 0;
std::shared_ptr<MediaSample> sample;
@@ -137,7 +138,7 @@
}};
EXPECT_EQ(callback->waitUntilFinished(), AMEDIA_OK);
- EXPECT_TRUE(transcoder.stop());
+ EXPECT_TRUE(transcoder->stop());
sampleConsumerThread.join();
}
@@ -148,11 +149,70 @@
std::shared_ptr<TestCallback> callback = std::make_shared<TestCallback>();
std::shared_ptr<AMediaFormat> nullFormat;
- VideoTrackTranscoder transcoder{callback};
- EXPECT_EQ(transcoder.configure(mMediaSampleReader, 0 /* trackIndex */, nullFormat),
+ auto transcoder = VideoTrackTranscoder::create(callback);
+ EXPECT_EQ(transcoder->configure(mMediaSampleReader, 0 /* trackIndex */, nullFormat),
AMEDIA_ERROR_INVALID_PARAMETER);
}
+TEST_F(VideoTrackTranscoderTests, LingeringEncoder) {
+ struct {
+ void wait() {
+ std::unique_lock<std::mutex> lock(mMutex);
+ while (!mSignaled) {
+ mCondition.wait(lock);
+ }
+ }
+
+ void signal() {
+ std::unique_lock<std::mutex> lock(mMutex);
+ mSignaled = true;
+ mCondition.notify_all();
+ }
+
+ std::mutex mMutex;
+ std::condition_variable mCondition;
+ bool mSignaled = false;
+ } semaphore;
+
+ auto callback = std::make_shared<TestCallback>();
+ auto transcoder = VideoTrackTranscoder::create(callback);
+
+ EXPECT_EQ(transcoder->configure(mMediaSampleReader, mTrackIndex, mDestinationFormat),
+ AMEDIA_OK);
+ ASSERT_TRUE(transcoder->start());
+
+ std::shared_ptr<MediaSampleQueue> outputQueue = transcoder->getOutputQueue();
+ std::vector<std::shared_ptr<MediaSample>> samples;
+ std::thread sampleConsumerThread([&outputQueue, &samples, &semaphore] {
+ std::shared_ptr<MediaSample> sample;
+ while (samples.size() < 10 && !outputQueue->dequeue(&sample)) {
+ ASSERT_NE(sample, nullptr);
+ samples.push_back(sample);
+
+ if (sample->info.flags & SAMPLE_FLAG_END_OF_STREAM) {
+ break;
+ }
+ sample.reset();
+ }
+
+ semaphore.signal();
+ });
+
+ // Wait for the encoder to output samples before stopping and releasing the transcoder.
+ semaphore.wait();
+
+ EXPECT_TRUE(transcoder->stop());
+ transcoder.reset();
+ sampleConsumerThread.join();
+
+ // Return buffers to the codec so that it can resume processing, but keep one buffer to avoid
+ // the codec being released.
+ samples.resize(1);
+
+ // Wait for async codec events.
+ std::this_thread::sleep_for(std::chrono::seconds(1));
+}
+
} // namespace android
int main(int argc, char** argv) {