Propagate playback time for video only cases in MediaCodecPlayer

This CL permanently attach the time update callback to the video
decoder, making two time update callbacks: one for audio and one
for video. When the audio stream is present (i.e. most of the time)
the result from the video callback is ignored.

BUG=407577

Review URL: https://codereview.chromium.org/1234003003

Cr-Commit-Position: refs/heads/master@{#339108}

5 files changed, 139 insertions, 30 deletions

diff --git a/media/base/android/media_codec_decoder_unittest.cc b/media/base/android/media_codec_decoder_unittest.cc
index 3c32250..39e6661 100644
--- a/media/base/android/media_codec_decoder_unittest.cc
+++ b/media/base/android/media_codec_decoder_unittest.cc
@@ -35,7 +35,7 @@ const base::TimeDelta kVideoFramePeriod = base::TimeDelta::FromMilliseconds(20);
 
 class AudioFactory : public TestDataFactory {
  public:
-  AudioFactory(const base::TimeDelta& duration);
+  AudioFactory(base::TimeDelta duration);
   DemuxerConfigs GetConfigs() const override;
 
  protected:
@@ -44,14 +44,14 @@ class AudioFactory : public TestDataFactory {
 
 class VideoFactory : public TestDataFactory {
  public:
-  VideoFactory(const base::TimeDelta& duration);
+  VideoFactory(base::TimeDelta duration);
   DemuxerConfigs GetConfigs() const override;
 
  protected:
   void ModifyAccessUnit(int index_in_chunk, AccessUnit* unit) override;
 };
 
-AudioFactory::AudioFactory(const base::TimeDelta& duration)
+AudioFactory::AudioFactory(base::TimeDelta duration)
     : TestDataFactory("aac-44100-packet-%d", duration, kAudioFramePeriod) {
 }
 
@@ -63,7 +63,7 @@ void AudioFactory::ModifyAccessUnit(int index_in_chunk, AccessUnit* unit) {
   unit->is_key_frame = true;
 }
 
-VideoFactory::VideoFactory(const base::TimeDelta& duration)
+VideoFactory::VideoFactory(base::TimeDelta duration)
     : TestDataFactory("h264-320x180-frame-%d", duration, kVideoFramePeriod) {
 }
 
@@ -82,14 +82,23 @@ void VideoFactory::ModifyAccessUnit(int index_in_chunk, AccessUnit* unit) {
   //
   // I keep the last PTS to be 3 for simplicity.
 
-  // Swap pts for second and third frames.
-  if (index_in_chunk == 1)  // second frame
-    unit->timestamp += frame_period_;
-  if (index_in_chunk == 2)  // third frame
-    unit->timestamp -= frame_period_;
-
-  if (index_in_chunk == 0)
-    unit->is_key_frame = true;
+  // Swap pts for second and third frames. Make first frame a key frame.
+  switch (index_in_chunk) {
+    case 0:  // first frame
+      unit->is_key_frame = true;
+      break;
+    case 1:  // second frame
+      unit->timestamp += frame_period_;
+      break;
+    case 2:  // third frame
+      unit->timestamp -= frame_period_;
+      break;
+    case 3:  // fourth frame, do not modify
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
 }
 
 }  // namespace (anonymous)
diff --git a/media/base/android/media_codec_player.cc b/media/base/android/media_codec_player.cc
index aa05fdf..da1fff1 100644
--- a/media/base/android/media_codec_player.cc
+++ b/media/base/android/media_codec_player.cc
@@ -448,6 +448,19 @@ void MediaCodecPlayer::OnTimeIntervalUpdate(DemuxerStream::Type type,
                                             base::TimeDelta last_buffered) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
 
+  DVLOG(2) << __FUNCTION__ << ": stream type:" << type << " [" << now_playing
+           << "," << last_buffered << "]";
+
+  // I assume that audio stream cannot be added after we get configs by
+  // OnDemuxerConfigsAvailable(), but that audio can finish early.
+
+  if (type == DemuxerStream::VIDEO) {
+    // Ignore video PTS if there is audio stream or if it's behind current
+    // time as set by audio stream.
+    if (!AudioFinished() || now_playing < interpolator_.GetInterpolatedTime())
+      return;
+  }
+
   interpolator_.SetBounds(now_playing, last_buffered);
 
   // Post to UI thread
@@ -687,7 +700,8 @@ void MediaCodecPlayer::CreateDecoders() {
       base::Bind(&MediaCodecPlayer::OnStarvation, media_weak_this_,
                  DemuxerStream::VIDEO),
       base::Bind(&MediaCodecPlayer::OnStopDone, media_weak_this_), error_cb_,
-      MediaCodecDecoder::SetTimeCallback(),  // null callback
+      base::Bind(&MediaCodecPlayer::OnTimeIntervalUpdate, media_weak_this_,
+                 DemuxerStream::VIDEO),
       base::Bind(&MediaCodecPlayer::OnVideoResolutionChanged, media_weak_this_),
       base::Bind(&MediaCodecPlayer::OnVideoCodecCreated, media_weak_this_)));
 }
diff --git a/media/base/android/media_codec_player_unittest.cc b/media/base/android/media_codec_player_unittest.cc
index bfd349b..c917c7f 100644
--- a/media/base/android/media_codec_player_unittest.cc
+++ b/media/base/android/media_codec_player_unittest.cc
@@ -12,6 +12,7 @@
 #include "media/base/android/test_data_factory.h"
 #include "media/base/android/test_statistics.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gl/android/surface_texture.h"
 
 namespace media {
 
@@ -39,6 +40,7 @@ namespace {
 const base::TimeDelta kDefaultTimeout = base::TimeDelta::FromMilliseconds(200);
 const base::TimeDelta kAudioFramePeriod =
     base::TimeDelta::FromSecondsD(1024.0 / 44100);  // 1024 samples @ 44100 Hz
+const base::TimeDelta kVideoFramePeriod = base::TimeDelta::FromMilliseconds(20);
 
 // Mock of MediaPlayerManager for testing purpose.
 
@@ -140,7 +142,7 @@ DemuxerConfigs CreateAudioVideoConfigs(const TestDataFactory* audio,
 
 class AudioFactory : public TestDataFactory {
  public:
-  AudioFactory(const base::TimeDelta& duration)
+  AudioFactory(base::TimeDelta duration)
       : TestDataFactory("aac-44100-packet-%d", duration, kAudioFramePeriod) {}
 
   DemuxerConfigs GetConfigs() const override {
@@ -153,6 +155,49 @@ class AudioFactory : public TestDataFactory {
   }
 };
 
+// VideoFactory creates a video stream from demuxer.
+
+class VideoFactory : public TestDataFactory {
+ public:
+  VideoFactory(base::TimeDelta duration)
+      : TestDataFactory("h264-320x180-frame-%d", duration, kVideoFramePeriod) {}
+
+  DemuxerConfigs GetConfigs() const override {
+    return TestDataFactory::CreateVideoConfigs(kCodecH264, duration_,
+                                               gfx::Size(320, 180));
+  }
+
+ protected:
+  void ModifyAccessUnit(int index_in_chunk, AccessUnit* unit) override {
+    // The frames are taken from High profile and some are B-frames.
+    // The first 4 frames appear in the file in the following order:
+    //
+    // Frames:             I P B P
+    // Decoding order:     0 1 2 3
+    // Presentation order: 0 2 1 4(3)
+    //
+    // I keep the last PTS to be 3 for simplicity.
+
+    // Swap pts for second and third frames. Make first frame a key frame.
+    switch (index_in_chunk) {
+      case 0:  // first frame
+        unit->is_key_frame = true;
+        break;
+      case 1:  // second frame
+        unit->timestamp += frame_period_;
+        break;
+      case 2:  // third frame
+        unit->timestamp -= frame_period_;
+        break;
+      case 3:  // fourth frame, do not modify
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+  }
+};
+
 // Mock of DemuxerAndroid for testing purpose.
 
 class MockDemuxerAndroid : public DemuxerAndroid {
@@ -211,7 +256,7 @@ void MockDemuxerAndroid::RequestDemuxerData(DemuxerStream::Type type) {
   bool created = false;
   if (type == DemuxerStream::AUDIO && audio_factory_)
     created = audio_factory_->CreateChunk(&chunk, &delay);
-  else if (type == DemuxerStream::VIDEO && audio_factory_)
+  else if (type == DemuxerStream::VIDEO && video_factory_)
     created = video_factory_->CreateChunk(&chunk, &delay);
 
   if (!created)
@@ -228,9 +273,10 @@ void MockDemuxerAndroid::RequestDemuxerData(DemuxerStream::Type type) {
 }
 
 void MockDemuxerAndroid::PostConfigs(const DemuxerConfigs& configs) {
-  DVLOG(1) << "MockDemuxerAndroid::" << __FUNCTION__;
   RUN_ON_MEDIA_THREAD(MockDemuxerAndroid, PostConfigs, configs);
 
+  DVLOG(1) << "MockDemuxerAndroid::" << __FUNCTION__;
+
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
 
   if (client_)
@@ -265,6 +311,7 @@ class MediaCodecPlayerTest : public testing::Test {
   typedef base::Callback<bool()> Predicate;
 
   void CreatePlayer();
+  void SetVideoSurface();
 
   // Waits for condition to become true or for timeout to expire.
   // Returns true if the condition becomes true.
@@ -274,6 +321,7 @@ class MediaCodecPlayerTest : public testing::Test {
   base::MessageLoop message_loop_;
   MockMediaPlayerManager manager_;
   MockDemuxerAndroid* demuxer_;  // owned by player_
+  scoped_refptr<gfx::SurfaceTexture> surface_texture_;
   MediaCodecPlayer* player_;     // raw pointer due to DeleteOnCorrectThread()
 
  private:
@@ -289,6 +337,11 @@ MediaCodecPlayerTest::MediaCodecPlayerTest()
     : demuxer_(new MockDemuxerAndroid()), player_(nullptr) {
 }
 
+MediaCodecPlayerTest::~MediaCodecPlayerTest() {
+  if (player_)
+    player_->DeleteOnCorrectThread();
+}
+
 void MediaCodecPlayerTest::CreatePlayer() {
   DCHECK(demuxer_);
   player_ = new MediaCodecPlayer(
@@ -301,9 +354,12 @@ void MediaCodecPlayerTest::CreatePlayer() {
   DCHECK(player_);
 }
 
-MediaCodecPlayerTest::~MediaCodecPlayerTest() {
-  if (player_)
-    player_->DeleteOnCorrectThread();
+void MediaCodecPlayerTest::SetVideoSurface() {
+  surface_texture_ = gfx::SurfaceTexture::Create(0);
+  gfx::ScopedJavaSurface surface(surface_texture_.get());
+
+  ASSERT_NE(nullptr, player_);
+  player_->SetVideoSurface(surface.Pass());
 }
 
 bool MediaCodecPlayerTest::WaitForCondition(const Predicate& condition,
@@ -399,7 +455,7 @@ TEST_F(MediaCodecPlayerTest, SetAudioVideoConfigsAfterPlayerCreation) {
   EXPECT_EQ(240, manager_.media_metadata_.height);
 }
 
-TEST_F(MediaCodecPlayerTest, PlayAudioTillCompletion) {
+TEST_F(MediaCodecPlayerTest, AudioPlayTillCompletion) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
   base::TimeDelta duration = base::TimeDelta::FromMilliseconds(1000);
@@ -432,4 +488,35 @@ TEST_F(MediaCodecPlayerTest, PlayAudioTillCompletion) {
   EXPECT_LT(duration - audio_pts_delay, manager_.pts_stat_.max());
 }
 
+TEST_F(MediaCodecPlayerTest, VideoPlayTillCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  base::TimeDelta duration = base::TimeDelta::FromMilliseconds(500);
+  base::TimeDelta timeout = base::TimeDelta::FromMilliseconds(1500);
+
+  demuxer_->SetVideoFactory(
+      scoped_ptr<VideoFactory>(new VideoFactory(duration)));
+
+  CreatePlayer();
+  SetVideoSurface();
+
+  // Wait till the player is initialized on media thread.
+  EXPECT_TRUE(WaitForCondition(base::Bind(&MockDemuxerAndroid::IsInitialized,
+                                          base::Unretained(demuxer_))));
+
+  // Post configuration after the player has been initialized.
+  demuxer_->PostInternalConfigs();
+
+  EXPECT_FALSE(manager_.IsPlaybackCompleted());
+
+  player_->Start();
+
+  EXPECT_TRUE(
+      WaitForCondition(base::Bind(&MockMediaPlayerManager::IsPlaybackCompleted,
+                                  base::Unretained(&manager_)),
+                       timeout));
+
+  EXPECT_LE(duration, manager_.pts_stat_.max());
+}
+
 }  // namespace media
diff --git a/media/base/android/test_data_factory.cc b/media/base/android/test_data_factory.cc
index 759c494..c53f8bc5e 100644
--- a/media/base/android/test_data_factory.cc
+++ b/media/base/android/test_data_factory.cc
@@ -11,9 +11,8 @@
 
 namespace media {
 
-DemuxerConfigs TestDataFactory::CreateAudioConfigs(
-    AudioCodec audio_codec,
-    const base::TimeDelta& duration) {
+DemuxerConfigs TestDataFactory::CreateAudioConfigs(AudioCodec audio_codec,
+                                                   base::TimeDelta duration) {
   DemuxerConfigs configs;
   configs.audio_codec = audio_codec;
   configs.audio_channels = 2;
@@ -45,7 +44,7 @@ DemuxerConfigs TestDataFactory::CreateAudioConfigs(
 
 DemuxerConfigs TestDataFactory::CreateVideoConfigs(
     VideoCodec video_codec,
-    const base::TimeDelta& duration,
+    base::TimeDelta duration,
     const gfx::Size& video_size) {
   DemuxerConfigs configs;
   configs.video_codec = video_codec;
@@ -57,8 +56,8 @@ DemuxerConfigs TestDataFactory::CreateVideoConfigs(
 }
 
 TestDataFactory::TestDataFactory(const char* file_name_template,
-                                 const base::TimeDelta& duration,
-                                 const base::TimeDelta& frame_period)
+                                 base::TimeDelta duration,
+                                 base::TimeDelta frame_period)
     : duration_(duration),
       frame_period_(frame_period),
       starvation_mode_(false) {
diff --git a/media/base/android/test_data_factory.h b/media/base/android/test_data_factory.h
index b186d75..0287e61 100644
--- a/media/base/android/test_data_factory.h
+++ b/media/base/android/test_data_factory.h
@@ -19,9 +19,9 @@ class TestDataFactory {
  public:
   // These methods return corresponding demuxer configs.
   static DemuxerConfigs CreateAudioConfigs(AudioCodec audio_codec,
-                                           const base::TimeDelta& duration);
+                                           base::TimeDelta duration);
   static DemuxerConfigs CreateVideoConfigs(VideoCodec video_codec,
-                                           const base::TimeDelta& duration,
+                                           base::TimeDelta duration,
                                            const gfx::Size& video_size);
 
   // Constructor calls |LoadPackets| to load packets from files.
@@ -33,8 +33,8 @@ class TestDataFactory {
   //             unit and stops.
   //   frame_period: PTS increment between units.
   TestDataFactory(const char* file_name_template,
-                  const base::TimeDelta& duration,
-                  const base::TimeDelta& frame_period);
+                  const base::TimeDelta duration,
+                  const base::TimeDelta frame_period);
   virtual ~TestDataFactory();
 
   // Returns demuxer configuration for this factory.