Remove FFmpegDemuxer::first_seek_hack_ and related unnecessary code.

The warning in WebMediaPlayerImpl was added in r23598 but has long since been true.

FFmpegDemuxer::first_seek_hack_ is due to Pipeline issuing a seek to the start timestamp as the final stage of initialization, however the demuxer doesn't require prerolling so we can safely skip it and remove the hack entirely.

TEST=media_unittests, http/tests/media/, any HTTP 200 chunked response

Review URL: https://chromiumcodereview.appspot.com/10024033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@131485 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 4 insertions, 43 deletions

diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index d3854e4..719f579 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -717,11 +717,12 @@ void Pipeline::InitializeTask(PipelineStatus last_stage_status) {
     PlaybackRateChangedTask(GetPlaybackRate());
     VolumeChangedTask(GetVolume());
 
-    // Fire the seek request to get the filters to preroll.
+    // Fire a seek request to get the renderers to preroll. We don't need to
+    // tell the demuxer to seek since it should already be at the start.
     seek_pending_ = true;
     SetState(kSeeking);
     seek_timestamp_ = demuxer_->GetStartTime();
-    DoSeek(seek_timestamp_);
+    OnDemuxerSeekDone(seek_timestamp_, PIPELINE_OK);
   }
 }
 
@@ -1312,7 +1313,7 @@ void Pipeline::OnDemuxerStopDone(const base::Closure& callback) {
 }
 
 void Pipeline::DoSeek(base::TimeDelta seek_timestamp) {
-  // TODO(acolwell) : We might be able to convert this if (demuxer_) into a
+  // TODO(acolwell): We might be able to convert this if (demuxer_) into a
   // DCHECK(). Further investigation is needed to make sure this won't introduce
   // a bug.
   if (demuxer_) {
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index 5827ff6..8ad3b1b 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -112,8 +112,6 @@ class PipelineTest : public ::testing::Test {
         .WillOnce(DoAll(SetDemuxerProperties(duration),
                         Invoke(&RunPipelineStatusCB2)));
     EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
-    EXPECT_CALL(*mocks_->demuxer(), Seek(mocks_->demuxer()->GetStartTime(), _))
-        .WillOnce(Invoke(&RunPipelineStatusCB2));
     EXPECT_CALL(*mocks_->demuxer(), Stop(_))
         .WillOnce(Invoke(&RunStopFilterCallback));
 
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 67448cb..f147a5f 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -288,7 +288,6 @@ FFmpegDemuxer::FFmpegDemuxer(
       last_read_bytes_(0),
       read_position_(0),
       bitrate_(0),
-      first_seek_hack_(true),
       start_time_(kNoTimestamp()),
       audio_disabled_(false) {
   DCHECK(message_loop_);
@@ -590,17 +589,6 @@ bool FFmpegDemuxer::IsSeekable() {
 void FFmpegDemuxer::SeekTask(base::TimeDelta time, const PipelineStatusCB& cb) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  // TODO(scherkus): remove this by separating Seek() from Flush() from
-  // Preroll() states (i.e., the implicit Seek(0) should really be a Preroll()).
-  if (first_seek_hack_) {
-    first_seek_hack_ = false;
-
-    if (time == start_time_) {
-      cb.Run(PIPELINE_OK);
-      return;
-    }
-  }
-
   // Tell streams to flush buffers due to seeking.
   StreamVector::iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 0fbe2c0..0815f62 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -162,14 +162,7 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer, public FFmpegURLProtocol {
   // Provide access to FFmpegDemuxerStream.
   MessageLoop* message_loop();
 
-  // For testing purposes.
-  void disable_first_seek_hack_for_testing() { first_seek_hack_ = false; }
-
  private:
-  // Only allow a factory to create this class.
-  friend class MockFFmpegDemuxer;
-  FRIEND_TEST_ALL_PREFIXES(FFmpegDemuxerTest, ProtocolRead);
-
   // Carries out initialization on the demuxer thread.
   void InitializeTask(DemuxerHost* host, const PipelineStatusCB& status_cb);
 
@@ -246,10 +239,6 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer, public FFmpegURLProtocol {
   // Derived bitrate after initialization has completed.
   int bitrate_;
 
-  // Used to skip the implicit "first seek" to avoid resetting FFmpeg's internal
-  // state.
-  bool first_seek_hack_;
-
   // The first timestamp of the opened media file. This is used to set the
   // starting clock value to match the timestamps in the media file. Default
   // is 0.
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index c75f34f..704fa03 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -69,10 +69,7 @@ class FFmpegDemuxerTest : public testing::Test {
         .WillRepeatedly(SaveArg<0>(&current_read_position_));
 
     CreateDataSource(name, disable_file_size);
-
-    // Create an FFmpegDemuxer with local data source.
     demuxer_ = new FFmpegDemuxer(&message_loop_, data_source_, true);
-    demuxer_->disable_first_seek_hack_for_testing();
   }
 
   MOCK_METHOD1(CheckPoint, void(int v));
diff --git a/webkit/media/webmediaplayer_impl.cc b/webkit/media/webmediaplayer_impl.cc
index 1d44526..41084a4 100644
--- a/webkit/media/webmediaplayer_impl.cc
+++ b/webkit/media/webmediaplayer_impl.cc
@@ -288,18 +288,6 @@ bool WebMediaPlayerImpl::supportsSave() const {
 void WebMediaPlayerImpl::seek(float seconds) {
   DCHECK_EQ(main_loop_, MessageLoop::current());
 
-  // WebKit fires a seek(0) at the very start, however pipeline already does a
-  // seek(0) internally.  Avoid doing seek(0) the second time because this will
-  // cause extra pre-rolling and will break servers without range request
-  // support.
-  //
-  // We still have to notify WebKit that time has changed otherwise
-  // HTMLMediaElement gets into an inconsistent state.
-  if (pipeline_->GetCurrentTime().ToInternalValue() == 0 && seconds == 0) {
-    GetClient()->timeChanged();
-    return;
-  }
-
   if (seeking_) {
     pending_seek_ = true;
     pending_seek_seconds_ = seconds;