diff options
| author | watk <watk@chromium.org> | 2016-03-21 17:56:59 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2016-03-22 00:58:54 +0000 |
| commit | 4c88276f8f1fdcb0a921bfb979e22cd2acf241bb (patch) | |
| tree | 0d71573df9b0520c8549442a7be77f3518da63bd /media | |
| parent | f1f689fb5b3ede5756b138e617ed7c879f181cec (diff) | |
| download | chromium_src-4c88276f8f1fdcb0a921bfb979e22cd2acf241bb.zip chromium_src-4c88276f8f1fdcb0a921bfb979e22cd2acf241bb.tar.gz chromium_src-4c88276f8f1fdcb0a921bfb979e22cd2acf241bb.tar.bz2 | |
Revert of media: Stop flushing the renderer for pipeline suspend (patchset #2 id:20001 of https://codereview.chromium.org/1815423002/ )
Reason for revert:
This hit a CHECK in PipelineIntegrationTest.SuspendWhilePlaying
[12876:12876:0321/173811:1285613567:FATAL:ffmpeg_demuxer.cc(592)] Check failed: read_cb_.is_null(). Overlapping reads are not supported
https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28valgrind%29%281%29/builds/41437
This was something we didn't consider and probably can't be relanded without a fair amount of work.
Original issue's description:
> media: Stop flushing the renderer for pipeline suspend
>
> Previously when performing a pipeline suspend we would flush the
> renderer before destructing it. This CL removes the flush because
> it's no longer necessary and has issues:
> 1) it adds ~100-150ms to suspend on a Galaxy Nexus (which makes
> a difference for fullscreen transitions), and
> 2) on <API level 18 devices it will cause AVDA to go into an
> error state when it handles the flush by creating a new
> MediaCodec without a valid output surface.
>
> BUG=596641,595545
> TEST=media_unittests
>
> Committed: https://crrev.com/9edddb9ded49023460a204675ee4e9bb763b9da7
> Cr-Commit-Position: refs/heads/master@{#382417}
TBR=sandersd@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=596641,595545
Review URL: https://codereview.chromium.org/1824923002
Cr-Commit-Position: refs/heads/master@{#382465}
Diffstat (limited to 'media')
| -rw-r--r-- | media/base/pipeline_impl.cc | 5 | ||||
| -rw-r--r-- | media/base/pipeline_impl_unittest.cc | 10 |
2 files changed, 15 insertions, 0 deletions
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc index d038af9..5f4705b 100644 --- a/media/base/pipeline_impl.cc +++ b/media/base/pipeline_impl.cc @@ -670,6 +670,11 @@ void PipelineImpl::SuspendTask(const PipelineStatusCB& suspend_cb) { if (text_renderer_) { fns.Push(base::Bind(&TextRenderer::Pause, base::Unretained(text_renderer_.get()))); + } + + fns.Push(base::Bind(&Renderer::Flush, base::Unretained(renderer_.get()))); + + if (text_renderer_) { fns.Push(base::Bind(&TextRenderer::Flush, base::Unretained(text_renderer_.get()))); } diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc index 49f69bd..d08ecf1 100644 --- a/media/base/pipeline_impl_unittest.cc +++ b/media/base/pipeline_impl_unittest.cc @@ -276,6 +276,11 @@ class PipelineImplTest : public ::testing::Test { void ExpectSuspend() { EXPECT_CALL(*renderer_, SetPlaybackRate(0)); + EXPECT_CALL(*renderer_, Flush(_)) + .WillOnce(DoAll( + SetBufferingState(&buffering_state_cb_, BUFFERING_HAVE_NOTHING), + RunClosure<0>())); + EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_NOTHING)); EXPECT_CALL(callbacks_, OnSuspend(PIPELINE_OK)); } @@ -1076,7 +1081,12 @@ class PipelineTeardownTest : public PipelineImplTest { base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)); EXPECT_CALL(*renderer_, SetPlaybackRate(0)); + EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_NOTHING)); EXPECT_CALL(callbacks_, OnSuspend(PIPELINE_OK)); + EXPECT_CALL(*renderer_, Flush(_)) + .WillOnce(DoAll( + SetBufferingState(&buffering_state_cb_, BUFFERING_HAVE_NOTHING), + RunClosure<0>())); if (state == kResuming) { if (stop_or_error == kStop) { EXPECT_CALL(*demuxer_, Seek(_, _)) |