Commit 6481ef3e authored by liberato@chromium.org's avatar liberato@chromium.org Committed by Commit Bot

Made MediaCodecAudioDecoder handle unsolicied EOS's.

Some devices seem to be sending multiple EOS output buffers to
MCAD, which doesn't handle them well.  This CL makes it possible
for a MediaCodecLoop client to signal an error when an EOS buffer
arrives, to transition MCL into the error state safely.

Bug: 818866
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ic40b2a10ee4d983be28211b18ee9268a379f13d5
Reviewed-on: https://chromium-review.googlesource.com/957642Reviewed-by: default avatarThomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542576}
parent 221d1b12
...@@ -279,7 +279,8 @@ bool MediaCodecLoop::ProcessOneOutputBuffer() { ...@@ -279,7 +279,8 @@ bool MediaCodecLoop::ProcessOneOutputBuffer() {
media_codec_->ReleaseOutputBuffer(out.index, false); media_codec_->ReleaseOutputBuffer(out.index, false);
client_->OnDecodedEos(out); if (!client_->OnDecodedEos(out))
SetState(STATE_ERROR);
} else { } else {
if (!client_->OnDecodedFrame(out)) if (!client_->OnDecodedFrame(out))
SetState(STATE_ERROR); SetState(STATE_ERROR);
......
...@@ -173,8 +173,9 @@ class MEDIA_EXPORT MediaCodecLoop { ...@@ -173,8 +173,9 @@ class MEDIA_EXPORT MediaCodecLoop {
// call back won't happen. // call back won't happen.
virtual void OnInputDataQueued(bool success) = 0; virtual void OnInputDataQueued(bool success) = 0;
// Called when an EOS buffer is dequeued from the output. // Called when an EOS buffer is dequeued from the output. If this returns
virtual void OnDecodedEos(const OutputBuffer& out) = 0; // false, then we transition to STATE_ERROR.
virtual bool OnDecodedEos(const OutputBuffer& out) = 0;
// Processes the output buffer after it comes from MediaCodec. The client // Processes the output buffer after it comes from MediaCodec. The client
// has the responsibility to release the codec buffer, though it doesn't // has the responsibility to release the codec buffer, though it doesn't
......
...@@ -39,7 +39,7 @@ class MockMediaCodecLoopClient : public StrictMock<MediaCodecLoop::Client> { ...@@ -39,7 +39,7 @@ class MockMediaCodecLoopClient : public StrictMock<MediaCodecLoop::Client> {
MOCK_CONST_METHOD0(IsAnyInputPending, bool()); MOCK_CONST_METHOD0(IsAnyInputPending, bool());
MOCK_METHOD0(ProvideInputData, MediaCodecLoop::InputData()); MOCK_METHOD0(ProvideInputData, MediaCodecLoop::InputData());
MOCK_METHOD1(OnInputDataQueued, void(bool)); MOCK_METHOD1(OnInputDataQueued, void(bool));
MOCK_METHOD1(OnDecodedEos, void(const MediaCodecLoop::OutputBuffer&)); MOCK_METHOD1(OnDecodedEos, bool(const MediaCodecLoop::OutputBuffer&));
MOCK_METHOD1(OnDecodedFrame, bool(const MediaCodecLoop::OutputBuffer&)); MOCK_METHOD1(OnDecodedFrame, bool(const MediaCodecLoop::OutputBuffer&));
MOCK_METHOD0(OnOutputFormatChanged, bool()); MOCK_METHOD0(OnOutputFormatChanged, bool());
MOCK_METHOD0(OnCodecLoopError, void()); MOCK_METHOD0(OnCodecLoopError, void());
...@@ -256,7 +256,7 @@ TEST_F(MediaCodecLoopTest, TestPendingWorkWithOutputBuffer) { ...@@ -256,7 +256,7 @@ TEST_F(MediaCodecLoopTest, TestPendingWorkWithOutputBuffer) {
} }
TEST_F(MediaCodecLoopTest, TestQueueEos) { TEST_F(MediaCodecLoopTest, TestQueueEos) {
// Test sending an EOS to MCL => MCB =dequeue output=> MCL . // Test sending an EOS to MCL => MCB =dequeue EOS=> MCL .
ConstructCodecLoop(); ConstructCodecLoop();
{ {
InSequence _s; InSequence _s;
...@@ -275,7 +275,7 @@ TEST_F(MediaCodecLoopTest, TestQueueEos) { ...@@ -275,7 +275,7 @@ TEST_F(MediaCodecLoopTest, TestQueueEos) {
EosOutputBuffer eos; EosOutputBuffer eos;
ExpectDequeueOutputBuffer(eos); ExpectDequeueOutputBuffer(eos);
EXPECT_CALL(Codec(), ReleaseOutputBuffer(eos.index, false)); EXPECT_CALL(Codec(), ReleaseOutputBuffer(eos.index, false));
EXPECT_CALL(*client_, OnDecodedEos(_)).Times(1); EXPECT_CALL(*client_, OnDecodedEos(_)).Times(1).WillOnce(Return(true));
// See TestUnqueuedEos. // See TestUnqueuedEos.
EXPECT_CALL(Codec(), DequeueOutputBuffer(_, _, _, _, _, _, _)) EXPECT_CALL(Codec(), DequeueOutputBuffer(_, _, _, _, _, _, _))
...@@ -286,6 +286,33 @@ TEST_F(MediaCodecLoopTest, TestQueueEos) { ...@@ -286,6 +286,33 @@ TEST_F(MediaCodecLoopTest, TestQueueEos) {
// Don't WaitUntilIdle() here. See TestUnqueuedEos. // Don't WaitUntilIdle() here. See TestUnqueuedEos.
} }
TEST_F(MediaCodecLoopTest, TestQueueEosFailure) {
// Test sending an EOS to MCL => MCB =dequeue EOS fails=> MCL error.
ConstructCodecLoop();
{
InSequence _s;
ExpectIsAnyInputPending(true);
int input_buffer_index = 123;
ExpectDequeueInputBuffer(input_buffer_index);
MediaCodecLoop::InputData data;
data.is_eos = true;
ExpectProvideInputData(data);
EXPECT_CALL(Codec(), QueueEOS(input_buffer_index));
ExpectInputDataQueued(true);
// Now send the EOS back on the output queue.
EosOutputBuffer eos;
ExpectDequeueOutputBuffer(eos);
EXPECT_CALL(Codec(), ReleaseOutputBuffer(eos.index, false));
EXPECT_CALL(*client_, OnDecodedEos(_)).Times(1).WillOnce(Return(false));
EXPECT_CALL(*client_, OnCodecLoopError()).Times(1);
}
codec_loop_->DoPendingWork();
// Don't WaitUntilIdle() here.
}
TEST_F(MediaCodecLoopTest, TestQueueInputData) { TEST_F(MediaCodecLoopTest, TestQueueInputData) {
// Send a buffer full of data into MCL and make sure that it gets queued with // Send a buffer full of data into MCL and make sure that it gets queued with
// MediaCodecBridge correctly. // MediaCodecBridge correctly.
......
...@@ -346,20 +346,29 @@ void MediaCodecAudioDecoder::OnCodecLoopError() { ...@@ -346,20 +346,29 @@ void MediaCodecAudioDecoder::OnCodecLoopError() {
ClearInputQueue(DecodeStatus::DECODE_ERROR); ClearInputQueue(DecodeStatus::DECODE_ERROR);
} }
void MediaCodecAudioDecoder::OnDecodedEos( bool MediaCodecAudioDecoder::OnDecodedEos(
const MediaCodecLoop::OutputBuffer& out) { const MediaCodecLoop::OutputBuffer& out) {
DVLOG(2) << __func__ << " pts:" << out.pts; DVLOG(2) << __func__ << " pts:" << out.pts;
// Rarely, we seem to get multiple EOSes or, possibly, unsolicited ones from
// MediaCodec. Just transition to the error state.
// https://crbug.com/818866
if (!input_queue_.size() || !input_queue_.front().first->end_of_stream()) {
LOG(WARNING) << "MCAD received unexpected eos";
return false;
}
// If we've transitioned into the error state, then we don't really know what // If we've transitioned into the error state, then we don't really know what
// to do. If we transitioned because of OnCodecError, then all of our // to do. If we transitioned because of OnCodecError, then all of our
// buffers have been returned anyway. Otherwise, it's unclear. Note that // buffers have been returned anyway. Otherwise, it's unclear. Note that
// MCL does not call us back after OnCodecError(), since it stops decoding. // MCL does not call us back after OnCodecError(), since it stops decoding.
// So, we shouldn't be in that state. So, just DCHECK here. // So, we shouldn't be in that state. So, just DCHECK here.
DCHECK_NE(state_, STATE_ERROR); DCHECK_NE(state_, STATE_ERROR);
DCHECK(input_queue_.size());
DCHECK(input_queue_.front().first->end_of_stream());
input_queue_.front().second.Run(DecodeStatus::OK); input_queue_.front().second.Run(DecodeStatus::OK);
input_queue_.pop_front(); input_queue_.pop_front();
return true;
} }
bool MediaCodecAudioDecoder::OnDecodedFrame( bool MediaCodecAudioDecoder::OnDecodedFrame(
......
...@@ -99,7 +99,7 @@ class MEDIA_EXPORT MediaCodecAudioDecoder : public AudioDecoder, ...@@ -99,7 +99,7 @@ class MEDIA_EXPORT MediaCodecAudioDecoder : public AudioDecoder,
bool IsAnyInputPending() const override; bool IsAnyInputPending() const override;
MediaCodecLoop::InputData ProvideInputData() override; MediaCodecLoop::InputData ProvideInputData() override;
void OnInputDataQueued(bool) override; void OnInputDataQueued(bool) override;
void OnDecodedEos(const MediaCodecLoop::OutputBuffer& out) override; bool OnDecodedEos(const MediaCodecLoop::OutputBuffer& out) override;
bool OnDecodedFrame(const MediaCodecLoop::OutputBuffer& out) override; bool OnDecodedFrame(const MediaCodecLoop::OutputBuffer& out) override;
bool OnOutputFormatChanged() override; bool OnOutputFormatChanged() override;
void OnCodecLoopError() override; void OnCodecLoopError() override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment