Fix UMA metrics for MediaSourcePlayer.

media/base/android/media_source_player.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/android/media_source_player.h"
 
 #include <limits>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 72 matching lines @@
     return;
   // Retry video decoder creation.
   RetryDecoderCreation(false, true);
 }
 
 void MediaSourcePlayer::ScheduleSeekEventAndStopDecoding(
     base::TimeDelta seek_time) {
   DVLOG(1) << __FUNCTION__ << "(" << seek_time.InSecondsF() << ")";
   DCHECK(!IsEventPending(SEEK_EVENT_PENDING));
 
+  // Need to stop here to prevent false reports while seek and drag.

[qinmin, 2015/11/18 18:12:27]

Do we need this? We should have captured this situation in
MediaDecoderCallback().

[Tima Vaisburd, 2015/11/18 18:31:13]

I think we do. During the seek-and-drag II did not see the MediaDecoderCallback
coming. We have many Starts in rapid sequence without StopAndReport, which leads
to another wrong long duration. And we need the first Start to take effect
instead of the last one because of starvations that can be "invisible" (come and
go, playback continues).

[qinmin, 2015/11/18 19:11:43]

Start() will automatically kick off the MediaDecoderJob to start requesting or
decoding data, wouldn't that leads to MediaDecoderCallback() later?

[Tima Vaisburd, 2015/11/18 20:02:12]

Yes, you are right, sorry I misled you. The bug was in the MediaStatistics where
I did wrong treatment of the unknown current time.

+  media_stat_->StopAndReport(GetCurrentTime());
+
   pending_seek_ = false;
 
   interpolator_.SetBounds(seek_time, seek_time);
 
   if (audio_decoder_job_->is_decoding())
     audio_decoder_job_->StopDecode();
   if (video_decoder_job_->is_decoding())
     video_decoder_job_->StopDecode();
 
   SetPendingEvent(SEEK_EVENT_PENDING);
@@ skipping 72 matching lines @@
   return std::min(interpolator_.GetInterpolatedTime(), duration_);
 }
 
 base::TimeDelta MediaSourcePlayer::GetDuration() {
   return duration_;
 }
 
 void MediaSourcePlayer::Release() {
   DVLOG(1) << __FUNCTION__;
 
+  media_stat_->StopAndReport(GetCurrentTime());
+
   audio_decoder_job_->ReleaseDecoderResources();
   video_decoder_job_->ReleaseDecoderResources();
 
   // Prevent player restart, including job re-creation attempts.
   playing_ = false;
 
   decoder_starvation_callback_.Cancel();
 
   DetachListener();
   on_decoder_resources_released_cb_.Run(player_id());
@@ skipping 277 matching lines @@
 
   bool is_clock_manager = is_audio || !HasAudio();
 
   if (is_clock_manager)
     decoder_starvation_callback_.Cancel();
 
   if (status == MEDIA_CODEC_ERROR) {
     DVLOG(1) << __FUNCTION__ << " : decode error";
     Release();
     manager()->OnError(player_id(), MEDIA_ERROR_DECODE);
-    media_stat_->StopAndReport(GetCurrentTime());
+    if (is_clock_manager)
+      media_stat_->StopAndReport(GetCurrentTime());
     return;
   }
 
   // Increment frame counts for UMA.
   if (current_presentation_timestamp != kNoTimestamp()) {
     FrameStatistics& frame_stats = is_audio ? media_stat_->audio_frame_stats()
                                             : media_stat_->video_frame_stats();
     frame_stats.IncrementFrameCount();
     if (is_late_frame)
       frame_stats.IncrementLateFrameCount();
   }
 
   DCHECK(!IsEventPending(PREFETCH_DONE_EVENT_PENDING));
 
   // Let |SEEK_EVENT_PENDING| (the highest priority event outside of
   // |PREFETCH_DONE_EVENT_PENDING|) preempt output EOS detection here. Process
   // any other pending events only after handling EOS detection.
   if (IsEventPending(SEEK_EVENT_PENDING)) {
     ProcessPendingEvents();
-    media_stat_->StopAndReport(GetCurrentTime());
+    // In case of Seek GetCurrentTime() already tells the time to seek to.
+    if (is_clock_manager)
+      media_stat_->StopAndReport(current_presentation_timestamp);
     return;
   }
 
   if ((status == MEDIA_CODEC_OK || status == MEDIA_CODEC_INPUT_END_OF_STREAM) &&
       is_clock_manager && current_presentation_timestamp != kNoTimestamp()) {
     UpdateTimestamps(current_presentation_timestamp,
                      max_presentation_timestamp);
   }
 
   if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM) {
     PlaybackCompleted(is_audio);
     if (is_clock_manager)
       interpolator_.StopInterpolating();
   }
 
   if (pending_event_ != NO_EVENT_PENDING) {
     ProcessPendingEvents();
     return;
   }
 
   if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM) {
-    media_stat_->StopAndReport(GetCurrentTime());
+    if (is_clock_manager)
+      media_stat_->StopAndReport(GetCurrentTime());
     return;
   }
 
   if (!playing_) {
     if (is_clock_manager)
       interpolator_.StopInterpolating();
 
-    media_stat_->StopAndReport(GetCurrentTime());
+    if (is_clock_manager)
+      media_stat_->StopAndReport(GetCurrentTime());
     return;
   }
 
   if (status == MEDIA_CODEC_NO_KEY) {
     if (key_added_while_decode_pending_) {
       DVLOG(2) << __FUNCTION__ << ": Key was added during decoding.";
       ResumePlaybackAfterKeyAdded();
     } else {
       is_waiting_for_key_ = true;
       manager()->OnWaitingForDecryptionKey(player_id());
-      media_stat_->StopAndReport(GetCurrentTime());
+      if (is_clock_manager)
+        media_stat_->StopAndReport(GetCurrentTime());
     }
     return;
   }
 
   // If |key_added_while_decode_pending_| is true and both audio and video
   // decoding succeeded, we should clear |key_added_while_decode_pending_| here.
   // But that would add more complexity into this function. If we don't clear it
   // here, the worst case would be we call ResumePlaybackAfterKeyAdded() when
   // we don't really have a new key. This should rarely happen and the
   // performance impact should be pretty small.
   // TODO(qinmin/xhwang): This class is complicated because we handle both audio
   // and video in one file. If we separate them, we should be able to remove a
   // lot of duplication.
 
   // If the status is MEDIA_CODEC_ABORT, stop decoding new data. The player is
   // in the middle of a seek or stop event and needs to wait for the IPCs to
   // come.
   if (status == MEDIA_CODEC_ABORT) {
-    media_stat_->StopAndReport(GetCurrentTime());
+    if (is_clock_manager)
+      media_stat_->StopAndReport(GetCurrentTime());
     return;
   }
 
   if (prerolling_ && IsPrerollFinished(is_audio)) {
     if (IsPrerollFinished(!is_audio)) {
       prerolling_ = false;
       StartInternal();
     }
     return;
   }
@@ skipping 263 matching lines @@
   is_waiting_for_key_ = false;
   key_added_while_decode_pending_ = false;
 
   // StartInternal() will trigger a prefetch, where in most cases we'll just
   // use previously received data.
   if (playing_)
     StartInternal();
 }
 
 }  // namespace media