Bug 162920 - [MSE] Expose additional MediaSource methods to MediaSourcePrivate
Summary: [MSE] Expose additional MediaSource methods to MediaSourcePrivate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrique Ocaña
URL:
Keywords:
Depends on:
Blocks: 157314
  Show dependency treegraph
 
Reported: 2016-10-04 10:46 PDT by Enrique Ocaña
Modified: 2016-10-26 01:50 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.29 KB, patch)
2016-10-04 10:49 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Call sequence of MediaSource duration change from MediaSourceGStreamer::durationChanged() (41.30 KB, image/png)
2016-10-09 09:37 PDT, Enrique Ocaña
no flags Details
Patch (3.25 KB, patch)
2016-10-16 12:12 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (3.25 KB, patch)
2016-10-26 01:24 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrique Ocaña 2016-10-04 10:46:00 PDT
The GStreamer MSE platform implementation needs to report duration changes and trigger monitorSourceBuffers() calls.
Comment 1 Enrique Ocaña 2016-10-04 10:49:35 PDT
Created attachment 290618 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2016-10-08 09:10:46 PDT
Comment on attachment 290618 [details]
Patch

For me this is a go unless Eric or Jer say otherwise.
Comment 3 Jer Noble 2016-10-09 08:02:28 PDT
(In reply to comment #2)
> Comment on attachment 290618 [details]
> Patch
> 
> For me this is a go unless Eric or Jer say otherwise.

This is fine with me. But I am curious why the MediaSourcePrivate would want to set the duration rather than its SourceBufferPrivate objects doing so.
Comment 4 Enrique Ocaña 2016-10-09 09:37:21 PDT
Created attachment 291046 [details]
Call sequence of MediaSource duration change from MediaSourceGStreamer::durationChanged()

(In reply to comment #3)

> This is fine with me. But I am curious why the MediaSourcePrivate would want
> to set the duration rather than its SourceBufferPrivate objects doing so.

MediaSourceGStreamer is our platform implementation of MediaSourcePrivate. MediaSourceGStreamer::durationChanged() is the one reporting the duration change to MediaPlayerPrivateGStreamerMSE (our player private) and the player reports it to MediaSource (seen as MediaSourcePrivateClient by it). See attached call hierarchy snapshot to get it graphically (levels go from callees to callers, two callers at the same level mean they're independent callers).

How would the call sequence be for the duration report via SourceBufferPrivate which you suggest?
Comment 5 Jer Noble 2016-10-09 13:47:06 PDT
Aha. I had thought that MediaSource was the only MediaSourcePrivateClient, as the point of the client interface is to avoid having the platform/ have knowledge of dom-level objects. In the Mac port, the MediaSourcePrivate and MediaPlayerPrivate objects talk directly to one another. So when the duration changes, SourceBufferPrivate tells SourceBuffer, who tells MediaSource, who tells MediaSourcePrivate, who tells MediaPlayerPrivate. 

We did it this way because the duration calculations all happen up in the dom-level MediaSource object. There's no reason why our Private classes have to calculate the duration. 

So you're welcome to keep using the client interfaces the way you do in this patch, but I'd recommend trying to re-use the dom-level code as much as possible; that way when other ports update that behavior according to newer versions of the spec, your port will. Rendition from that.
Comment 6 Enrique Ocaña 2016-10-16 12:12:56 PDT
Created attachment 291772 [details]
Patch
Comment 7 Enrique Ocaña 2016-10-26 01:24:19 PDT
Created attachment 292902 [details]
Patch
Comment 8 Enrique Ocaña 2016-10-26 01:50:01 PDT
Comment on attachment 292902 [details]
Patch

Clearing flags on attachment: 292902

Committed r207889: <http://trac.webkit.org/changeset/207889>
Comment 9 Enrique Ocaña 2016-10-26 01:50:09 PDT
All reviewed patches have been landed.  Closing bug.