summaryrefslogtreecommitdiffstats
path: root/media
diff options
context:
space:
mode:
authorwatk <watk@chromium.org>2016-03-21 17:56:59 -0700
committerCommit bot <commit-bot@chromium.org>2016-03-22 00:58:54 +0000
commit4c88276f8f1fdcb0a921bfb979e22cd2acf241bb (patch)
tree0d71573df9b0520c8549442a7be77f3518da63bd /media
parentf1f689fb5b3ede5756b138e617ed7c879f181cec (diff)
downloadchromium_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.cc5
-rw-r--r--media/base/pipeline_impl_unittest.cc10
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(_, _))