Allow media track switching.

media/renderers/renderer_impl_unittest.cc

Index: media/renderers/renderer_impl_unittest.cc
diff --git a/media/renderers/renderer_impl_unittest.cc b/media/renderers/renderer_impl_unittest.cc
index bbbaa2c27b3410558ac34600c65d297ee3769ed9..6f592cb19fe24b2166b79fd5fc731643f17c3870 100644
--- a/media/renderers/renderer_impl_unittest.cc
+++ b/media/renderers/renderer_impl_unittest.cc
@@ -32,6 +32,10 @@ namespace media {
 
 const int64_t kStartPlayingTimeInMs = 100;
 
+ACTION_P2(SetBool, var, value) {
+  *var = value;
+}
+
 ACTION_P2(SetBufferingState, renderer_client, buffering_state) {
   (*renderer_client)->OnBufferingStateChange(buffering_state);
 }
@@ -226,10 +230,7 @@ class RendererImplTest : public ::testing::Test {
     base::RunLoop().RunUntilIdle();
   }
 
-  void Flush(bool underflowed) {
-    if (!underflowed)
-      EXPECT_CALL(time_source_, StopTicking());
-
+  void SetFlushExpectationsForAVRenderers() {
     if (audio_stream_) {
       EXPECT_CALL(*audio_renderer_, Flush(_))
           .WillOnce(DoAll(SetBufferingState(&audio_renderer_client_,
@@ -243,7 +244,13 @@ class RendererImplTest : public ::testing::Test {
                                             BUFFERING_HAVE_NOTHING),
                           RunClosure<0>()));
     }
+  }
 
+  void Flush(bool underflowed) {
+    if (!underflowed)
+      EXPECT_CALL(time_source_, StopTicking());
+
+    SetFlushExpectationsForAVRenderers();
     EXPECT_CALL(callbacks_, OnFlushed());
 
     renderer_impl_->Flush(
@@ -851,4 +858,199 @@ TEST_F(RendererImplTest, PostponedStreamStatusNotificationHandling) {
   base::RunLoop().Run();
 }
 
+// Verify that a RendererImpl::Flush gets postponed until after stream status
+// change handling is completed.
+TEST_F(RendererImplTest, FlushDuringAudioReinit) {
+  CreateAudioAndVideoStream();
+
+  StreamStatusChangeCB stream_status_change_cb;
+  EXPECT_CALL(*demuxer_, SetStreamStatusChangeCB(_))
+      .WillOnce(SaveArg<0>(&stream_status_change_cb));
+  SetAudioRendererInitializeExpectations(PIPELINE_OK);
+  SetVideoRendererInitializeExpectations(PIPELINE_OK);
+  InitializeAndExpect(PIPELINE_OK);
+  Play();
+
+  EXPECT_CALL(time_source_, StopTicking()).Times(testing::AnyNumber());
+  base::Closure audio_renderer_flush_cb;
+  EXPECT_CALL(*audio_renderer_, Flush(_))
+      .WillOnce(SaveArg<0>(&audio_renderer_flush_cb));
+  EXPECT_CALL(*audio_renderer_, StartPlaying())
+      .Times(1)
+      .WillOnce(
+          SetBufferingState(&audio_renderer_client_, BUFFERING_HAVE_ENOUGH));
+
+  // This should start flushing the audio renderer (due to audio stream status
+  // change) and should populate the |audio_renderer_flush_cb|.
+  stream_status_change_cb.Run(audio_stream_.get(), false, base::TimeDelta());
+  EXPECT_TRUE(audio_renderer_flush_cb);
+  base::RunLoop().RunUntilIdle();
+
+  bool flush_done = false;
+
+  // Now that audio stream change is being handled the RendererImpl::Flush
+  // should be postponed, instead of being executed immediately.
+  EXPECT_CALL(callbacks_, OnFlushed()).WillOnce(SetBool(&flush_done, true));
+  renderer_impl_->Flush(
+      base::Bind(&CallbackHelper::OnFlushed, base::Unretained(&callbacks_)));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(flush_done);
+
+  // The renderer_impl_->Flush invoked above should proceed after the first
+  // audio renderer flush (initiated by the stream status change) completes.
+  SetFlushExpectationsForAVRenderers();
+  audio_renderer_flush_cb.Run();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(flush_done);
+}
+
+TEST_F(RendererImplTest, FlushDuringVideoReinit) {
+  CreateAudioAndVideoStream();
+
+  StreamStatusChangeCB stream_status_change_cb;
+  EXPECT_CALL(*demuxer_, SetStreamStatusChangeCB(_))
+      .WillOnce(SaveArg<0>(&stream_status_change_cb));
+  SetAudioRendererInitializeExpectations(PIPELINE_OK);
+  SetVideoRendererInitializeExpectations(PIPELINE_OK);
+  InitializeAndExpect(PIPELINE_OK);
+  Play();
+
+  EXPECT_CALL(time_source_, StopTicking()).Times(testing::AnyNumber());
+  base::Closure video_renderer_flush_cb;
+  EXPECT_CALL(*video_renderer_, Flush(_))
+      .WillOnce(SaveArg<0>(&video_renderer_flush_cb));
+  EXPECT_CALL(*video_renderer_, StartPlayingFrom(_))
+      .Times(1)
+      .WillOnce(
+          SetBufferingState(&video_renderer_client_, BUFFERING_HAVE_ENOUGH));
+
+  // This should start flushing the video renderer (due to video stream status
+  // change) and should populate the |video_renderer_flush_cb|.
+  stream_status_change_cb.Run(video_stream_.get(), false, base::TimeDelta());
+  EXPECT_TRUE(video_renderer_flush_cb);
+  base::RunLoop().RunUntilIdle();
+
+  bool flush_done = false;
+
+  // Now that video stream change is being handled the RendererImpl::Flush
+  // should be postponed, instead of being executed immediately.
+  EXPECT_CALL(callbacks_, OnFlushed()).WillOnce(SetBool(&flush_done, true));
+  renderer_impl_->Flush(
+      base::Bind(&CallbackHelper::OnFlushed, base::Unretained(&callbacks_)));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(flush_done);
+
+  // The renderer_impl_->Flush invoked above should proceed after the first
+  // video renderer flush (initiated by the stream status change) completes.
+  SetFlushExpectationsForAVRenderers();
+  video_renderer_flush_cb.Run();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(flush_done);
+}
+
+TEST_F(RendererImplTest, AudioTrackSwitchDuringFlush) {
+  CreateAudioAndVideoStream();
+  std::unique_ptr<StrictMock<MockDemuxerStream>> primary_audio_stream =
+      std::move(audio_stream_);
+  CreateAudioStream();
+  std::unique_ptr<StrictMock<MockDemuxerStream>> secondary_audio_stream =
+      std::move(audio_stream_);
+  audio_stream_ = std::move(primary_audio_stream);
+
+  StreamStatusChangeCB stream_status_change_cb;
+  EXPECT_CALL(*demuxer_, SetStreamStatusChangeCB(_))
+      .WillOnce(SaveArg<0>(&stream_status_change_cb));
+  SetAudioRendererInitializeExpectations(PIPELINE_OK);
+  SetVideoRendererInitializeExpectations(PIPELINE_OK);
+  InitializeAndExpect(PIPELINE_OK);
+  Play();
+
+  EXPECT_CALL(time_source_, StopTicking()).Times(testing::AnyNumber());
+  EXPECT_CALL(*video_renderer_, Flush(_))
+      .WillRepeatedly(DoAll(
+          SetBufferingState(&video_renderer_client_, BUFFERING_HAVE_NOTHING),
+          RunClosure<0>()));
+
+  // Initiate RendererImpl::Flush, but postpone its completion by not calling
+  // audio renderer flush callback right away, i.e. pretending audio renderer
+  // flush takes a while.
+  base::Closure audio_renderer_flush_cb;
+  EXPECT_CALL(*audio_renderer_, Flush(_))
+      .WillOnce(SaveArg<0>(&audio_renderer_flush_cb));
+  EXPECT_CALL(callbacks_, OnFlushed());
+  renderer_impl_->Flush(
+      base::Bind(&CallbackHelper::OnFlushed, base::Unretained(&callbacks_)));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(audio_renderer_flush_cb);
+
+  // Now, while the RendererImpl::Flush is pending, perform an audio track
+  // switch. The handling of the track switch will be postponed until after
+  // RendererImpl::Flush completes.
+  stream_status_change_cb.Run(audio_stream_.get(), false, base::TimeDelta());
+  stream_status_change_cb.Run(secondary_audio_stream.get(), true,
+                              base::TimeDelta());
+
+  // Ensure that audio track switch occurs after Flush by verifying that the
+  // audio renderer is reinitialized with the secondary audio stream.
+  EXPECT_CALL(*audio_renderer_,
+              Initialize(secondary_audio_stream.get(), _, _, _));
+
+  audio_renderer_client_->OnBufferingStateChange(BUFFERING_HAVE_NOTHING);
+  audio_renderer_flush_cb.Run();
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(RendererImplTest, VideoTrackSwitchDuringFlush) {
+  CreateAudioAndVideoStream();
+  std::unique_ptr<StrictMock<MockDemuxerStream>> primary_video_stream =
+      std::move(video_stream_);
+  CreateVideoStream();
+  std::unique_ptr<StrictMock<MockDemuxerStream>> secondary_video_stream =
+      std::move(video_stream_);
+  video_stream_ = std::move(primary_video_stream);
+
+  StreamStatusChangeCB stream_status_change_cb;
+  EXPECT_CALL(*demuxer_, SetStreamStatusChangeCB(_))
+      .WillOnce(SaveArg<0>(&stream_status_change_cb));
+  SetAudioRendererInitializeExpectations(PIPELINE_OK);
+  SetVideoRendererInitializeExpectations(PIPELINE_OK);
+  InitializeAndExpect(PIPELINE_OK);
+  Play();
+
+  EXPECT_CALL(time_source_, StopTicking()).Times(testing::AnyNumber());
+  EXPECT_CALL(*video_renderer_, OnTimeStopped()).Times(testing::AnyNumber());
+  EXPECT_CALL(*audio_renderer_, Flush(_))
+      .WillRepeatedly(DoAll(
+          SetBufferingState(&audio_renderer_client_, BUFFERING_HAVE_NOTHING),
+          RunClosure<0>()));
+
+  // Initiate RendererImpl::Flush, but postpone its completion by not calling
+  // video renderer flush callback right away, i.e. pretending video renderer
+  // flush takes a while.
+  base::Closure video_renderer_flush_cb;
+  EXPECT_CALL(*video_renderer_, Flush(_))
+      .WillOnce(SaveArg<0>(&video_renderer_flush_cb));
+  EXPECT_CALL(callbacks_, OnFlushed());
+  renderer_impl_->Flush(
+      base::Bind(&CallbackHelper::OnFlushed, base::Unretained(&callbacks_)));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(video_renderer_flush_cb);
+
+  // Now, while the RendererImpl::Flush is pending, perform a video track
+  // switch. The handling of the track switch will be postponed until after
+  // RendererImpl::Flush completes.
+  stream_status_change_cb.Run(video_stream_.get(), false, base::TimeDelta());
+  stream_status_change_cb.Run(secondary_video_stream.get(), true,
+                              base::TimeDelta());
+
+  // Ensure that video track switch occurs after Flush by verifying that the
+  // video renderer is reinitialized with the secondary video stream.
+  EXPECT_CALL(*video_renderer_,
+              Initialize(secondary_video_stream.get(), _, _, _, _));
+
+  video_renderer_client_->OnBufferingStateChange(BUFFERING_HAVE_NOTHING);
+  video_renderer_flush_cb.Run();
+  base::RunLoop().RunUntilIdle();
+}

[xhwang, 2017/04/10 18:57:30]

Can you add test cases for:
- stream switch in "FLUSHED" state

[servolk, 2017/04/10 20:21:22]

The *TrackSwitchDuringFlush actually include testing of both states. For example
in VideoTrackSwitchDuringFlush we do the track switch on lines 1042-1043, while
the renderer is still in the FLUSHING state (due to the renderer_impl_->Flush
invocation at line 1034 and due to a pending video_renderer_flush_cb). So the
track switch gets done in the FLUSHING state and postponed initially. Later,
when we complete the video renderer flush on line 1052, it allows the pending
Flush from line 1034 to complete and triggers processing of the pending track
status changes. Now the pending status changes initiated at lines 1042-1043 will
run again, but this time the renderer_impl_ is in the FLUSHED state, which allow
the stream switch to proceed and call the video_renderer_ Initialize with the
secondary video stream satisfying the EXPECT_CALL at line 1048.

[xhwang, 2017/04/11 00:29:41]

Thanks, this is not obvious from the test name. Could you please add a comment?

[servolk, 2017/04/11 01:14:05]

Done.

+
 }  // namespace media