Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(217)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: media/base/pipeline_unittest.cc

Issue 10837206: Rewrite media::Pipeline state transition machinery and simplify shutdown. (Closed) Base URL: svn://chrome-svn/chrome/trunk/src
Patch Set: one more time Created 8 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« media/base/pipeline.cc ('K') | « media/base/pipeline.cc ('k') | media/base/serial_runner.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/base/pipeline_unittest.cc
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index a91ad322802c14bc4b45e69544bcfb9395131916..ce3066d1d59df6ee4f020c053e83606dfabb4ca3 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -113,24 +113,12 @@ class PipelineTest : public ::testing::Test {
EXPECT_CALL(*mocks_->demuxer(), Stop(_))
.WillOnce(RunClosure());
- // TODO(scherkus): Don't pause+flush on shutdown,
- // see http://crbug.com/110228
if (audio_stream_) {
Ami GONE FROM CHROMIUM 2012/09/02 20:29:29 braces no longer necessary (here and below).
scherkus (not reviewing) 2012/09/05 15:04:17 Done.
- EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
- .WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
- .WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->audio_renderer(), Stop(_))
- .WillOnce(RunClosure());
+ EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
}
if (video_stream_) {
- EXPECT_CALL(*mocks_->video_renderer(), Pause(_))
- .WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Flush(_))
- .WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Stop(_))
- .WillOnce(RunClosure());
+ EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
}
}
@@ -266,6 +254,7 @@ class PipelineTest : public ::testing::Test {
// Every filter should receive a call to Seek().
EXPECT_CALL(*mocks_->demuxer(), Seek(seek_time, _))
.WillOnce(RunPipelineStatusCB());
+ EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(_));
if (audio_stream_) {
EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
@@ -274,6 +263,8 @@ class PipelineTest : public ::testing::Test {
.WillOnce(RunClosure());
EXPECT_CALL(*mocks_->audio_renderer(), Preroll(seek_time, _))
.WillOnce(RunPipelineStatusCB());
+ EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(_));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(_));
EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
.WillOnce(RunClosure());
}
@@ -285,6 +276,7 @@ class PipelineTest : public ::testing::Test {
.WillOnce(RunClosure());
EXPECT_CALL(*mocks_->video_renderer(), Preroll(seek_time, _))
.WillOnce(RunPipelineStatusCB());
+ EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(_));
EXPECT_CALL(*mocks_->video_renderer(), Play(_))
.WillOnce(RunClosure());
}
@@ -710,7 +702,7 @@ TEST_F(PipelineTest, ErrorDuringSeek) {
base::TimeDelta seek_time = base::TimeDelta::FromSeconds(5);
- // Seek() isn't called as the demuxer errors out first.
+ // Preroll() isn't called as the demuxer errors out first.
EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
.WillOnce(RunClosure());
EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
@@ -866,6 +858,9 @@ TEST_F(PipelineTest, AudioTimeUpdateDuringSeek) {
.WillOnce(RunClosure());
EXPECT_CALL(*mocks_->audio_renderer(), Preroll(seek_time, _))
.WillOnce(RunPipelineStatusCB());
+ EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(_));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(_));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(_));
EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
.WillOnce(RunClosure());
@@ -972,32 +967,15 @@ class PipelineTeardownTest : public PipelineTest {
case kFlushing:
case kSeeking:
case kPrerolling:
- case kStarting: {
+ case kStarting:
DoInitialize(state, stop_or_error);
-
- InSequence s;
- if (stop_or_error == kStop) {
- ExpectSeekStop(state);
- } else {
- ExpectSeekError(state);
- }
- DoSeek();
+ DoSeek(state, stop_or_error);
break;
- }
- case kPlaying: {
+ case kPlaying:
DoInitialize(state, stop_or_error);
-
- InSequence s;
- if (stop_or_error == kStop) {
- ExpectStop();
- DoStop();
- } else {
- ExpectPlaybackError();
- DoPlaybackError();
- }
+ DoStopOrError(stop_or_error);
break;
- }
}
}
@@ -1111,17 +1089,16 @@ class PipelineTeardownTest : public PipelineTest {
EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kHaveMetadata));
// If we get here it's a successful initialization.
- EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
- EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f));
- EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
-
- EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
-
EXPECT_CALL(*mocks_->audio_renderer(), Preroll(base::TimeDelta(), _))
.WillOnce(RunPipelineStatusCB());
EXPECT_CALL(*mocks_->video_renderer(), Preroll(base::TimeDelta(), _))
.WillOnce(RunPipelineStatusCB());
+ EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f));
+ EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
+
EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
.WillOnce(RunClosure());
EXPECT_CALL(*mocks_->video_renderer(), Play(_))
@@ -1133,107 +1110,89 @@ class PipelineTeardownTest : public PipelineTest {
return status;
}
- void ExpectSeekStop(TeardownState state) {
- base::Closure stop_cb = base::Bind(
- &CallbackHelper::OnStop, base::Unretained(&callbacks_));
-
- if (state == kPausing) {
- EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
- .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
- } else {
- EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
- }
-
- EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
-
- if (state == kFlushing) {
- EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
- .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
- } else {
- EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
- }
-
- EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
+ void DoSeek(TeardownState state, StopOrError stop_or_error) {
+ InSequence s;
+ PipelineStatus status = SetSeekExpectations(state, stop_or_error);
- if (state == kSeeking) {
- EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
- .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
- } else {
- EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
- .WillOnce(RunPipelineStatusCB());
- }
+ EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
+ EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
+ EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
+ EXPECT_CALL(callbacks_, OnSeek(status));
- if (state == kPrerolling) {
- EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
- .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
- } else {
- EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
- .WillOnce(RunPipelineStatusCB());
+ if (status == PIPELINE_OK) {
+ EXPECT_CALL(callbacks_, OnStop());
}
- EXPECT_CALL(*mocks_->video_renderer(), Preroll(_, _))
- .WillOnce(RunPipelineStatusCB());
-
- if (state == kStarting) {
- EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
- .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
- } else {
- EXPECT_CALL(*mocks_->audio_renderer(), Play(_)).WillOnce(RunClosure());
- }
-
- EXPECT_CALL(*mocks_->video_renderer(), Play(_)).WillOnce(RunClosure());
-
- EXPECT_CALL(callbacks_, OnBufferingState(Pipeline::kPrerollCompleted));
- EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
- ExpectStop();
+ pipeline_->Seek(base::TimeDelta::FromSeconds(10), base::Bind(
+ &CallbackHelper::OnSeek, base::Unretained(&callbacks_)));
+ message_loop_.RunAllPending();
}
- void ExpectSeekError(TeardownState state) {
- SetSeekErrorExpectations(state);
-
- // Executed after the error is raised.
- EXPECT_CALL(callbacks_, OnSeek(PIPELINE_ERROR_READ));
- EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
- }
+ PipelineStatus SetSeekExpectations(TeardownState state,
+ StopOrError stop_or_error) {
+ PipelineStatus status = PIPELINE_OK;
+ base::Closure stop_cb = base::Bind(
+ &CallbackHelper::OnStop, base::Unretained(&callbacks_));
- void SetSeekErrorExpectations(TeardownState state) {
if (state == kPausing) {
- EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
- .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
- RunClosure()));
- EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
- return;
+ if (stop_or_error == kStop) {
+ EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
+ .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+ } else {
+ status = PIPELINE_ERROR_READ;
+ EXPECT_CALL(*mocks_->audio_renderer(), Pause(_))
+ .WillOnce(DoAll(SetError(pipeline_, status), RunClosure()));
+ }
+
+ return status;
}
EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
if (state == kFlushing) {
- EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
- .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
- RunClosure()));
- EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
- return;
+ if (stop_or_error == kStop) {
+ EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
+ .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+ } else {
+ status = PIPELINE_ERROR_READ;
+ EXPECT_CALL(*mocks_->audio_renderer(), Flush(_))
+ .WillOnce(DoAll(SetError(pipeline_, status), RunClosure()));
+ }
+
+ return status;
}
EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
if (state == kSeeking) {
- EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
- .WillOnce(RunPipelineStatusCBWithStatus(PIPELINE_ERROR_READ));
- return;
+ if (stop_or_error == kStop) {
+ EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+ .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
+ } else {
+ status = PIPELINE_ERROR_READ;
+ EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
+ .WillOnce(RunPipelineStatusCBWithStatus(status));
+ }
+
+ return status;
}
EXPECT_CALL(*mocks_->demuxer(), Seek(_, _))
.WillOnce(RunPipelineStatusCB());
if (state == kPrerolling) {
- EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
- .WillOnce(RunPipelineStatusCBWithStatus(PIPELINE_ERROR_READ));
- return;
+ if (stop_or_error == kStop) {
+ EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
+ .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunPipelineStatusCB()));
+ } else {
+ status = PIPELINE_ERROR_READ;
+ EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
+ .WillOnce(RunPipelineStatusCBWithStatus(status));
+ }
+
+ return status;
}
EXPECT_CALL(*mocks_->audio_renderer(), Preroll(_, _))
@@ -1241,57 +1200,46 @@ class PipelineTeardownTest : public PipelineTest {
EXPECT_CALL(*mocks_->video_renderer(), Preroll(_, _))
.WillOnce(RunPipelineStatusCB());
+ // Playback rate and volume are updated prior to starting.
+ EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetPlaybackRate(0.0f));
+ EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
+ EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
+
if (state == kStarting) {
- EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
- .WillOnce(DoAll(SetError(pipeline_, PIPELINE_ERROR_READ),
- RunClosure()));
- EXPECT_CALL(*mocks_->video_renderer(), Play(_)).WillOnce(RunClosure());
- return;
+ if (stop_or_error == kStop) {
+ EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
+ .WillOnce(DoAll(Stop(pipeline_, stop_cb), RunClosure()));
+ } else {
+ status = PIPELINE_ERROR_READ;
+ EXPECT_CALL(*mocks_->audio_renderer(), Play(_))
+ .WillOnce(DoAll(SetError(pipeline_, status), RunClosure()));
+ }
+ return status;
}
- NOTREACHED() << "Unexpected TeardownState: " << state;
+ EXPECT_CALL(*mocks_->audio_renderer(), Play(_)).WillOnce(RunClosure());
+ EXPECT_CALL(*mocks_->video_renderer(), Play(_)).WillOnce(RunClosure());
+ NOTREACHED() << "WTF";
acolwell GONE FROM CHROMIUM 2012/09/05 13:22:22 Remove WTF or replace with a more helpful comment.
scherkus (not reviewing) 2012/09/05 15:04:17 whoops haha
+ return status;
}
- void ExpectStop() {
- // TODO(scherkus): Don't pause+flush, see http://crbug.com/110228
- EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
-
- EXPECT_CALL(callbacks_, OnStop());
- }
+ void DoStopOrError(StopOrError stop_or_error) {
+ InSequence s;
- void ExpectPlaybackError() {
- // TODO(scherkus): Don't pause+flush, see http://crbug.com/110228
- EXPECT_CALL(*mocks_->audio_renderer(), Pause(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Pause(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->audio_renderer(), Flush(_)).WillOnce(RunClosure());
- EXPECT_CALL(*mocks_->video_renderer(), Flush(_)).WillOnce(RunClosure());
EXPECT_CALL(*mocks_->demuxer(), Stop(_)).WillOnce(RunClosure());
EXPECT_CALL(*mocks_->audio_renderer(), Stop(_)).WillOnce(RunClosure());
EXPECT_CALL(*mocks_->video_renderer(), Stop(_)).WillOnce(RunClosure());
- EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
- }
-
- void DoSeek() {
- pipeline_->Seek(base::TimeDelta::FromSeconds(10), base::Bind(
- &CallbackHelper::OnSeek, base::Unretained(&callbacks_)));
- message_loop_.RunAllPending();
- }
-
- void DoStop() {
- pipeline_->Stop(base::Bind(
- &CallbackHelper::OnStop, base::Unretained(&callbacks_)));
- message_loop_.RunAllPending();
- }
+ if (stop_or_error == kStop) {
+ EXPECT_CALL(callbacks_, OnStop());
+ pipeline_->Stop(base::Bind(
+ &CallbackHelper::OnStop, base::Unretained(&callbacks_)));
+ } else {
+ EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
+ pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
+ }
- void DoPlaybackError() {
- pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
message_loop_.RunAllPending();
}
« media/base/pipeline.cc ('K') | « media/base/pipeline.cc ('k') | media/base/serial_runner.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698