Fix playback / seek issues (DF241115-08183)

Change-Id: I07b56993b6a74a21d95f3320b40342069096f110

diff --git a/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.cc b/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.cc
index 6e904e20adffca7c1045ac1760fe68e43f38c7c4..f61d0a477ec8eb5e43fa1cac0b13542308db6e79 100644 (file)
--- a/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.cc
+++ b/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.cc
@@ -33,8 +33,9 @@ const base::TimeDelta kBufferQueueReadDelay = base::Milliseconds(100);
 // the time stamp of the buffered data.
 const base::TimeDelta kMinWaitTimeForPlayback = base::Milliseconds(500);
 
-// The delay time to check buffering and decide operations.
-const base::TimeDelta kOperationDecisionDelay = base::Milliseconds(500);
+// The value above which packets will be discarded if they're too far away from
+// the seek position.
+const base::TimeDelta kMaxSeekDiff = base::Milliseconds(2000);
 
 player_buffer_size_t GetMaxAudioBufferSize() {
   return kPlayerAudioBufferSize;
@@ -398,15 +399,13 @@ void MediaPlayerESPlusPlayer::Release() {
   expected_seek_ = false;
   seek_position_ = base::TimeDelta();
   pending_seek_ = false;
-  pending_seek_position_ = base::TimeDelta();
+  pending_seek_position_ = base::TimeDelta::Max();
   should_set_playback_rate_ = false;
 
   ClearBufferQueue(DemuxerStream::VIDEO);
   ClearBufferQueue(DemuxerStream::AUDIO);
   sink_ = nullptr;
 
-  StopOperationForDataTimer();
-
   if (GetPlayerState() == ESPLUSPLAYER_STATE_NONE) {
     LOG_ID(ERROR, player_id_)
         << "(" << static_cast<void*>(this) << ") " << __func__
@@ -484,9 +483,6 @@ void MediaPlayerESPlusPlayer::Pause(bool is_media_related_action) {
   if (!is_media_related_action)
     is_paused_ = true;
 
-  // To decide operation is not needed in pause.
-  StopOperationForDataTimer();
-
   LOG_ID(INFO, player_id_) << "(" << static_cast<void*>(this) << ") "
                            << __func__ << " is_buffering : " << is_buffering_
                            << " is_media_action : " << is_media_related_action;
@@ -601,6 +597,7 @@ void MediaPlayerESPlusPlayer::Seek(base::TimeDelta time,
   ClearBufferQueue(DemuxerStream::VIDEO);
   ClearBufferQueue(DemuxerStream::AUDIO);
 
+  buffer_observer_->ResetBufferStatus();
   seek_cb_ = std::move(seek_cb);
 
   SeekInternal(time);
@@ -620,7 +617,7 @@ void MediaPlayerESPlusPlayer::SeekInternal(base::TimeDelta time) {
   is_readytoseek_ = false;
   seek_position_ = time;
   pending_seek_ = false;
-  pending_seek_position_ = base::TimeDelta();
+  pending_seek_position_ = base::TimeDelta::Max();
   if (seek_cb_)
     std::move(seek_cb_).Run();
 }
@@ -630,6 +627,10 @@ void MediaPlayerESPlusPlayer::Flush(base::OnceClosure flush_cb) {
   if (GetPlayerState() < ESPLUSPLAYER_STATE_READY) {
     LOG_ID(INFO, player_id_)
         << "player is not ready, state:" << GetString(GetPlayerState());
+
+    ReportBufferingStateIfNeeded(BUFFERING_HAVE_NOTHING,
+                                 BUFFERING_CHANGE_REASON_UNKNOWN);
+
     if (flush_cb)
       std::move(flush_cb).Run();
     return;
@@ -646,6 +647,7 @@ void MediaPlayerESPlusPlayer::Flush(base::OnceClosure flush_cb) {
   UpdateBufferedDtsDifference();
 
   expected_seek_ = true;
+
   buffer_observer_->ResetBufferStatus();
 
   std::move(flush_cb).Run();
@@ -892,18 +894,15 @@ void MediaPlayerESPlusPlayer::ReadBuffer(DemuxerStream::Type type) {
   if (!ReadFromBufferQueue(type))
     return;
 
+  PerformOperationForData();
+
   // Avoid unnecessary or redundant read requests.
   if (!ShouldFeed(type) || ReadRequested(type) || IsEos(type) ||
       (is_prepared_ && is_paused_ && !is_seeking_)) {
     return;
   }
 
-  // If the playing statue is expected,
-  // keep running a timer to decide play or pause with data.
-  if (!is_paused_) {
-    StartOperationForDataTimer();
-  }
-
+  // If there are more packets in the queue, process them...
   if (!GetBufferQueue(type).empty()) {
     ReadBuffer(type);
     return;
@@ -968,8 +967,34 @@ bool MediaPlayerESPlusPlayer::ReadFromBufferQueue(DemuxerStream::Type type) {
                                << ") player is not prepared ignore eos";
     } else
       status = SubmitEosPacket(type);
-  } else
+  } else {
+    // If we're expecting a seek (i.e. we've received a Flush()), then we need
+    // to discard buffers, otherwise there is a chance that the demux buffer
+    // is filled and the new data required to seek is not received.
+    if (expected_seek_ && is_prepared_) {
+      LOG_ID(INFO, player_id_) << "Discard buffers while expected_seek_";
+      GetBufferQueue(type).pop_front();
+      PostReadBuffer(type);
+      return true;
+    }
+
+    if (is_seeking_) {
+      // If we're seeking, and the data received is *not* for the current seek
+      // point, then discard it, or it can throw an error in the lower levels
+      // (causing an OnError() callback and video to stop).
+      base::TimeDelta diff = buffer->timestamp() - seek_position_;
+
+      if (diff.magnitude() > kMaxSeekDiff) {
+        LOG_ID(WARNING, player_id_) << "is_seeking_ buffer timestamp too far "
+                                    << "from seek position, discarding";
+        GetBufferQueue(type).pop_front();
+        PostReadBuffer(type);
+        return true;
+      }
+    }
+
     status = SubmitEsPacket(type, buffer);
+  }
 
   if (status == ESPLUSPLAYER_SUBMIT_STATUS_FULL) {
     // If buffer is full, try again after a while.
@@ -1026,7 +1051,7 @@ esplusplayer_submit_status MediaPlayerESPlusPlayer::SubmitEsPacket(
                            << ", is_key_frame: " << buffer->is_key_frame()
                            << ", this: " << this;
 
-  // This filed only set when PushMediaPacket
+  // This filled only set when PushMediaPacket
   packet.matroska_color_info = nullptr;
   esplusplayer_submit_status status =
       esplusplayer_submit_packet(esplayer_, &packet);
@@ -1064,8 +1089,13 @@ void MediaPlayerESPlusPlayer::OnBufferingStatusChanged(DemuxerStream::Type type,
   LOG_ID(INFO, player_id_) << __func__ << " "
                            << DemuxerStream::GetTypeName(type) << " : "
                            << GetString(status);
+
   if (expected_seek_ && is_prepared_) {
-    LOG_ID(INFO, player_id_) << "Ignore the buffer updates while seek";
+    // We need to wait for the Seek() call before we start processing buffers...
+    LOG_ID(INFO, player_id_) << "Ignore the buffer updates while expected seek";
+    if (GetBufferQueue(type).size()) {
+      GetBufferQueue(type).pop_front();
+    }
     return;
   }
 
@@ -1080,7 +1110,7 @@ void MediaPlayerESPlusPlayer::OnBufferingStatusChanged(DemuxerStream::Type type,
     case kBufferNormal:
       if (is_seeking_ && is_readytoseek_ == false) {
         LOG_ID(INFO, player_id_)
-            << "Ignore the buffer status before read to seek";
+            << "Ignore the buffer status before ready to seek";
       } else {
         is_buffering_ = true;
         SetShouldFeed(type, true);
@@ -1141,7 +1171,9 @@ void MediaPlayerESPlusPlayer::PerformOperationForData() {
       ReportBufferingStateIfNeeded(BUFFERING_HAVE_ENOUGH,
                                    BUFFERING_CHANGE_REASON_UNKNOWN);
       is_paused_by_buffering_ = false;
-      Play();
+      if (!is_paused_) {
+        Play();
+      }
       break;
     case OperationForData::PAUSE:
       ReportBufferingStateIfNeeded(BUFFERING_HAVE_NOTHING,
@@ -1149,6 +1181,8 @@ void MediaPlayerESPlusPlayer::PerformOperationForData() {
       is_paused_by_buffering_ = true;
       Pause(true);
       break;
+    default:
+      break;
   }
 }
 
@@ -1209,10 +1243,14 @@ MediaPlayerESPlusPlayer::GetOperationForData() {
     bool should_pause = false;
     if (has_video_media_type &&
         current_position >= last_frames_.get<DemuxerStream::VIDEO>().first) {
+      ReportBufferingStateIfNeeded(BUFFERING_HAVE_NOTHING,
+                                   BUFFERING_CHANGE_REASON_UNKNOWN);
       should_pause = true;
     }
     if (has_audio_media_type &&
         current_position >= last_frames_.get<DemuxerStream::AUDIO>().first) {
+      ReportBufferingStateIfNeeded(BUFFERING_HAVE_NOTHING,
+                                   BUFFERING_CHANGE_REASON_UNKNOWN);
       should_pause = true;
     }
 
@@ -1225,20 +1263,6 @@ MediaPlayerESPlusPlayer::GetOperationForData() {
   return OperationForData::NONE;
 }
 
-void MediaPlayerESPlusPlayer::StartOperationForDataTimer() {
-  if (!operation_for_data_timer_.IsRunning()) {
-    operation_for_data_timer_.Start(
-        FROM_HERE, kOperationDecisionDelay, this,
-        &MediaPlayerESPlusPlayer::PerformOperationForData);
-  }
-}
-
-void MediaPlayerESPlusPlayer::StopOperationForDataTimer() {
-  if (operation_for_data_timer_.IsRunning()) {
-    operation_for_data_timer_.Stop();
-  }
-}
-
 void MediaPlayerESPlusPlayer::OnReadyToPrepare(
     const esplusplayer_stream_type stream_type) {
   if (!task_runner_->BelongsToCurrentThread()) {

diff --git a/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.h b/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.h
index 2a9b059585af4f9fa69c2f0cb31fe40d7da990af..4b2a1e86acfc4dee031d8784713b874985df2595 100644 (file)
--- a/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.h
+++ b/tizen_src/chromium_impl/media/filters/media_player_esplusplayer.h
@@ -204,8 +204,6 @@ class MEDIA_EXPORT MediaPlayerESPlusPlayer : public MediaPlayerTizen {
 
   void PerformOperationForData();
   OperationForData GetOperationForData();
-  void StartOperationForDataTimer();
-  void StopOperationForDataTimer();
 
   ElementryStream& GetElementryStream(DemuxerStream::Type type);
   const ElementryStream& GetElementryStream(DemuxerStream::Type type) const;
@@ -253,8 +251,6 @@ class MEDIA_EXPORT MediaPlayerESPlusPlayer : public MediaPlayerTizen {
   std::array<ElementryStream, kElementryStreamCount> elementry_stream_;
   std::unique_ptr<BufferObserver> buffer_observer_;
 
-  base::RepeatingTimer operation_for_data_timer_;
-
   AppBufferStatus buffer_status_{kBufferNone, kBufferNone};
 
   base::WeakPtrFactory<MediaPlayerESPlusPlayer> weak_factory_{this};