MSE: Fix logic bugs with high precision duration

media/blink/webmediaplayer_impl.cc

 // Copyright 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/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 678 matching lines @@
 
   return seeking_;
 }
 
 double WebMediaPlayerImpl::duration() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   if (ready_state_ == WebMediaPlayer::ReadyStateHaveNothing)
     return std::numeric_limits<double>::quiet_NaN();
 
-  return GetPipelineDuration();
+  // Use duration from ChunkDemuxer when present. MSE allows users to specify
+  // duration as a double. This propagates to the rest of the pipeline as a
+  // TimeDelta with potentially reduced precision (limited to Microseconds).
+  // ChunkDemuxer returns the full-precision user-specified double. This ensures
+  // users can "get" the exact duration they "set".
+  if (chunk_demuxer_) {
+    return chunk_demuxer_->GetDuration();

[wolenetz, 2016/12/16 00:56:51]

Is WMPI::duration() ever queried in a code path where earlier in the stack the
ChunkDemuxer lock is still acquired? I seem to recall an ancient double-lock
potential around this.

[chcunningham, 2017/01/03 17:48:05]

Not that I'm aware of. I did some searching and couldn't find anything. This
would have to be something like an event bubbled up from ChunkDemuxer. The main
avenue for that is the demuxer "client", which is Pipeline::RenderWrapper, which
does a good job of consistently posting things to the main thread for them to
bubble up to WMPI.

+  }

[mlamouri (slow - plz ping), 2016/12/15 10:20:51]

style: no { }

[chcunningham, 2017/01/03 17:48:05]

Done.

+
+  base::TimeDelta pipeline_duration = pipeline_.GetMediaDuration();
+  return (pipeline_duration == kInfiniteDuration)

[mlamouri (slow - plz ping), 2016/12/15 10:20:51]

nit: () are not needed

[chcunningham, 2017/01/03 17:48:05]

Done.

+             ? std::numeric_limits<double>::infinity()
+             : pipeline_duration.InSecondsF();
 }
 
 double WebMediaPlayerImpl::timelineOffset() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   if (pipeline_metadata_.timeline_offset.is_null())
     return std::numeric_limits<double>::quiet_NaN();
 
   return pipeline_metadata_.timeline_offset.ToJsTime();
 }
@@ skipping 942 matching lines @@
   highest_ready_state_ = std::max(highest_ready_state_, ready_state_);
 
   // Always notify to ensure client has the latest value.
   client_->readyStateChanged();
 }
 
 blink::WebAudioSourceProvider* WebMediaPlayerImpl::getAudioSourceProvider() {
   return audio_source_provider_.get();
 }
 
-double WebMediaPlayerImpl::GetPipelineDuration() const {
-  base::TimeDelta duration = pipeline_.GetMediaDuration();
-
-  // Return positive infinity if the resource is unbounded.
-  // http://www.whatwg.org/specs/web-apps/current-work/multipage/video.html#dom-media-duration
-  if (duration == kInfiniteDuration)
-    return std::numeric_limits<double>::infinity();
-
-  return duration.InSecondsF();
-}
-
 static void GetCurrentFrameAndSignal(
     VideoFrameCompositor* compositor,
     scoped_refptr<VideoFrame>* video_frame_out,
     base::WaitableEvent* event) {
   TRACE_EVENT0("media", "GetCurrentFrameAndSignal");
   *video_frame_out = compositor->GetCurrentFrameAndUpdateIfStale();
   event->Signal();
 }
 
 scoped_refptr<VideoFrame>
@@ skipping 297 matching lines @@
     watch_time_reporter_->OnShown();
 }
 
 bool WebMediaPlayerImpl::IsHidden() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   return delegate_ && delegate_->IsHidden();
 }
 
 }  // namespace media