Fix buffered range updates in media::Pipeline

media/base/pipeline.h

 // Copyright (c) 2012 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.
 
 #ifndef MEDIA_BASE_PIPELINE_H_
 #define MEDIA_BASE_PIPELINE_H_
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ skipping 227 matching lines @@
   void SetState(State next_state);
 
   static const char* GetStateString(State state);
   State GetNextState() const;
 
   // Helper method that runs & resets |seek_cb_| and resets |seek_timestamp_|
   // and |seek_pending_|.
   void FinishSeek();
 
   // DemuxerHost implementaion.
-  void AddBufferedTimeRange(base::TimeDelta start,
-                            base::TimeDelta end) override;
+  void OnBufferedTimeRangesChanged(
+      const Ranges<base::TimeDelta>& ranges) override;
   void SetDuration(base::TimeDelta duration) override;
   void OnDemuxerError(PipelineStatus error) override;
   void AddTextStream(DemuxerStream* text_stream,
                      const TextTrackConfig& config) override;
   void RemoveTextStream(DemuxerStream* text_stream) override;
 
   // Callback executed when a rendering error happened, initiating the teardown
   // sequence.
   void OnError(PipelineStatus error);
 
@@ skipping 82 matching lines @@
 
   // Lock used to serialize access for the following data members.
   mutable base::Lock lock_;
 
   // Whether or not the pipeline is running.
   bool running_;
 
   // Amount of available buffered data as reported by |demuxer_|.
   Ranges<base::TimeDelta> buffered_time_ranges_;
 
-  // True when AddBufferedTimeRange() has been called more recently than
+  // True when OnBufferedTimeRangesChanged() has been called more recently than
   // DidLoadingProgress().

[wolenetz, 2016/01/06 19:28:41]

I'm not sure this has the same semantic meaning. If MSE web app explicitly
removes some previously buffered range(s), does that truly mean loading has
progressed?

[servolk, 2016/01/06 21:05:05]

I assumed that the HTMLMediaElement 'progress' event was a notification for the
apps that HTMLMediaElement::buffered ranges have changed. But now I checked the
spec
(http://www.w3.org/TR/2011/WD-html5-20110405/video.html#event-media-progress)
and it says "[Progress event is dispatched when...] The user agent is fetching
media data."
But if we don't fire the 'progress' event here, is there an alternative way to
notify apps that buffered ranges have changed?

[wolenetz, 2016/01/14 19:25:53]

Hmm. The spec also is more clear about when 'progress' is queued to be fired:
[In the resource fetch algorithm, which "Every 350ms (±200ms) or for every byte
received, whichever is least frequent, queue a task to fire a simple event named
progress at the element."
[Also,] Once the entire media resource has been fetched (but potentially before
any of it has been decoded)
Queue a task to fire a simple event named progress at the media element.

I suspect that range removal (either by MSE GC on Append or by MSE explicit
removal by app) is not ever in scope of normal HTML5 src=, hence the 'progress'
event really means just that fetching is in progress or has completed. Explicit
range removal or GC are both app-directed (the latter by append), and
'updateend' is the result of both, so the app knows if [potential] removal is
done and buffered ranges may have changed.

For Chrome MSE, any successful range removal occurs in a state where the
MediaSource is 'open' (not ended, for example), whereby the normal HTML5 src=
'progress' event is queued to fire every 350ms+/-200ms, just like what might
happen on the result of an MSE append or a src= continuing fetch. So, I think
exposing an MSE range removal (or addition; e.g. any buffered range change) as a
'didLoadingProgress == true' seems fine, since 'progress' is going to be fired
every 350+/200ms regardless until after the MediaSource has transitioned to
'ended'.

tl;dr: I've answered my original query in this thread, and I don't think any
further change here is needed at this point.

   bool did_loading_progress_;
 
   // Current volume level (from 0.0f to 1.0f).  This value is set immediately
   // via SetVolume() and a task is dispatched on the task runner to notify the
   // filters.
   float volume_;
 
   // Current playback rate (>= 0.0).  This value is set immediately via
   // SetPlaybackRate() and a task is dispatched on the task runner to notify
   // the filters.
@@ skipping 65 matching lines @@
 
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<Pipeline> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Pipeline);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_PIPELINE_H_