Add a segment-metadata TextTrack that contains cues for the segments currently in the buffer #976

mjneil · 2017-01-24T22:50:40Z

Description

Currently, we do not expose or track segment information for the segments that are currently in the actual buffer. Being able to know which segment or rendition is currently being rendered on screen has been a much requested feature. This addition allows users to listen to cuechange events on the track and see the segment information for the currently rendered segment in the active cues list.

Specific Changes proposed

Adds a new TextTrack to the tech with the label segment-metadata
The Cues in the track directly correspond to the current state of the buffer at a segment level
As segments are appended to the buffer, a new cue is added to the track that has start and end times equal to the segment. If the segment being appended overlaps already buffered content, we remove any overlapping cues from the track so that the cues in the track are always the most up to date.
whenever data is removed from the buffer, we remove any cues that also fall inside the removed area

Requirements Checklist

Feature implemented / Bug fixed
Unit Tests updated or fixed
Docs/guides updated
Reviewed by Two Core Contributors

imbcmdth · 2017-01-30T23:16:34Z

src/master-playlist-controller.js

+
+    // for now we don't want the audio segment loader to also add cues to the segment
+    // metadata track
+    delete segmentLoaderOptions.segmentMetadataTrack;


I think it would be better to create two different objects.

Perhaps using Object.create to make the mainSegmentLoader_'s object with segmentMetadataTrack added to the base object.

dmlap · 2017-02-01T19:10:31Z

src/segment-loader.js

+      end
+    };
+    const Cue = window.WebKitDataCue || window.VTTCue;
+    const data = JSON.stringify(value);


It might be more useful to just merge these properties onto the created cue object.

do you mean if say you wanted the uri, accessing it should look like cue.uri instead of cue.value.uri?

gesinger · 2017-02-01T19:12:47Z

src/segment-loader.js

+   * @method addSegmentMetadataCue_
+   */
+  addSegmentMetadataCue_(segmentInfo) {
+    if (!this.segmentMetadataTrack_ || !segmentInfo) {


I don't think we have to be guarding against these.

You're right with not needing a check for segmentInfo but i think for check for the track is still needed since both the main and audio segment loaders are instances of this class, but the audio segment loader doesnt have a segment metadata track, so we just want this function to be a noop in that case

Good call on checking for segmentMetadataTrack_

gesinger · 2017-02-01T19:17:56Z

src/segment-loader.js

+    const cue = new Cue(start, end, data);
+
+    value.data = data;
+    cue.value = value;


So cue ends up with a stringified value for its text, then has a value property which has the value object, including the stringified value?

Yes and no. It mainly depends on the const Cue = window.WebKitDataCue || window.VTTCue; line. I was following what is done in contrib-media-sources when adding id3 tags, but WebKitDataCue is only in safari right now, and there are some small differences between WebKitDataCue and VTTCue. Mainly, WebKitDataCue does not have a text property but instead a value. For id3 tags this value property has a key and data property usually. My goal with these lines was to minimize the differences between the two cues so that you can just use cue.value in all the browsers. After having to explain this, it may just be better to only use VTTCue and not jump through all these hoops

I think keeping Safari support is a good idea (just in case). Maybe a comment and/or some kind of helper method would help.

Well VTTCue is supported in safari, its just that WebKitDataCue is only supported in safari. Is it best practice to use these types of cues for certain purposes, or are they more general purpose and its not "wrong" to just use the VTTCue for safari as well?

Deciding to stick with what we do for id3 metadata tags since these cues are another form of metadata

I think it's ok to do here since it's fairly simple and we're creating metadata tracks.

gesinger · 2017-02-01T19:23:52Z

test/segment-loader.test.js

+    // verify stats
+    assert.equal(loader.mediaBytesTransferred, 30, '10 bytes');
+    assert.equal(loader.mediaRequests, 3, '1 request');
+  });


Would also be worth testing the behavior with overlapping times from a different timeline.

Adding a test for overlapping segments is definitely a good call. I don't think being on a different timeline makes a different and needs to be tested. At the point of creating the cues, the start and end times have been converted to display time, so the non-congruent timestamps across discontinuities are not a factor

gesinger · 2017-02-03T18:31:20Z

README.md

+### Segment Metadata
+You can get metadata about the segments currently in the buffer by using the `segment-metadata`
+text track. You can get the metadata of the currently rendered segment by looking at the
+tracks `activeCues` array. The metadata will be attached to the `cue.value` property and


gesinger · 2017-02-03T18:34:00Z

README.md

+
+```javascript
+cue.value = {
+  uri: The Segment uri,


Usually for JS blocks it's good to try to keep it as standard JS. Maybe something like:

cue.value = { uri, // segment uri ... }; // or: let data = cue.value; data.uri; // the segment uri ...

gesinger · 2017-02-14T19:17:51Z

README.md

+```javascript
+cue.value = {
+  uri: The Segment uri,
+  timeline: Timeline of the segment,


Might be useful to say what the timeline is

gesinger · 2017-02-14T19:20:30Z

src/segment-loader.js

+      return;
+    }
+
+    const playlist = segmentInfo.playlist;


Can get rid of this and just use segmentInfo.playlist.uri below

gesinger · 2017-03-04T01:01:50Z

Tested, but with the same code I can't get it working on IE11/Windows 7 (not even getting cuechange events from the segmentMetadataTrack), whereas it works even for Flash (and HTML) on Chrome/Mac.

forbesjo · 2017-03-14T21:05:00Z

Looks good to me

mjneil added the enhancement label Jan 24, 2017

imbcmdth reviewed Jan 30, 2017

View reviewed changes

mjneil force-pushed the seg-metadata branch 2 times, most recently from d19ef4d to e490c7f Compare February 1, 2017 17:24

dmlap reviewed Feb 1, 2017

View reviewed changes

gesinger reviewed Feb 1, 2017

View reviewed changes

mjneil assigned gesinger Feb 8, 2017

gesinger reviewed Feb 14, 2017

View reviewed changes

mjneil force-pushed the seg-metadata branch 2 times, most recently from b5ad752 to bac2f7b Compare February 21, 2017 19:56

gesinger approved these changes Feb 21, 2017

View reviewed changes

gesinger added the needs:testing label Feb 21, 2017

mjneil added 5 commits February 22, 2017 10:59

add buffered segment information to a text track as cues

a0571b9

fix tests

051c1e1

use default segment loader options and merge in differing properties

ddd1026

remove redundent cue data and update test

ad23bb3

update readme

a461a64

mjneil force-pushed the seg-metadata branch from bac2f7b to 04f4bb3 Compare February 22, 2017 15:59

update readme

386fceb

mjneil force-pushed the seg-metadata branch from 04f4bb3 to 386fceb Compare February 22, 2017 16:06

imbcmdth assigned forbesjo Mar 13, 2017

mjneil merged commit ab9a398 into videojs:master Mar 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a segment-metadata TextTrack that contains cues for the segments currently in the buffer #976

Add a segment-metadata TextTrack that contains cues for the segments currently in the buffer #976

mjneil commented Jan 24, 2017 •

edited by imbcmdth

Loading

imbcmdth Jan 30, 2017

dmlap Feb 1, 2017

mjneil Feb 2, 2017

gesinger Feb 1, 2017

mjneil Feb 1, 2017

gesinger Feb 1, 2017

gesinger Feb 1, 2017

mjneil Feb 1, 2017

gesinger Feb 1, 2017

mjneil Feb 1, 2017

mjneil Feb 2, 2017

gkatsev Feb 2, 2017

gesinger Feb 1, 2017

mjneil Feb 1, 2017

gesinger Feb 1, 2017

gesinger Feb 3, 2017

gesinger Feb 3, 2017

gesinger Feb 14, 2017

gesinger Feb 14, 2017

gesinger commented Mar 4, 2017

forbesjo commented Mar 14, 2017

Add a segment-metadata TextTrack that contains cues for the segments currently in the buffer #976

Add a segment-metadata TextTrack that contains cues for the segments currently in the buffer #976

Conversation

mjneil commented Jan 24, 2017 • edited by imbcmdth Loading

Description

Specific Changes proposed

Requirements Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gesinger commented Mar 4, 2017

forbesjo commented Mar 14, 2017

mjneil commented Jan 24, 2017 •

edited by imbcmdth

Loading