Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: stats for timeToLoadedData, appendsToLoadedData, mainAppendsToLoadedData, audioAppendsToLoadedData, and mediaAppends #1106

Merged
merged 19 commits into from
May 27, 2021
Merged

feat: stats for timeToLoadedData, appendsToLoadedData, mainAppendsToLoadedData, audioAppendsToLoadedData, and mediaAppends #1106

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
dae964e
feat: timeToFirstFrame, appendsToFirstFrame, and mediaAppends
brandonocasey Mar 26, 2021
1de5821
split up audio/main appends
brandonocasey Mar 26, 2021
ef33818
add tests
brandonocasey Mar 29, 2021
4b2ecd0
switch to loadeddata
brandonocasey Mar 30, 2021
ce4bf99
Merge remote-tracking branch 'origin/main' into feat/append-stats
brandonocasey Mar 30, 2021
d75374e
Merge remote-tracking branch 'origin/main' into feat/append-stats
brandonocasey Mar 30, 2021
7540d10
switch to timeupdate, add tests
brandonocasey Mar 30, 2021
51c7b10
Merge branch 'main' into feat/append-stats
brandonocasey Mar 30, 2021
4d9c882
loadeddata and play/loadstart
brandonocasey Mar 30, 2021
25b4889
Merge branch 'main' into feat/append-stats
brandonocasey Apr 7, 2021
d4b22dc
rename stats from FirstFrame -> LoadedData
brandonocasey Apr 7, 2021
9cd6e0f
Merge branch 'main' into feat/append-stats
brandonocasey Apr 8, 2021
eb017aa
Merge branch 'main' into feat/append-stats
brandonocasey Apr 8, 2021
c48c3bb
Merge branch 'main' into feat/append-stats
brandonocasey Apr 23, 2021
1f7852b
Merge branch 'main' into feat/append-stats
brandonocasey Apr 26, 2021
70155e7
Merge branch 'main' into feat/append-stats
brandonocasey May 25, 2021
6cd2015
Merge branch 'main' into feat/append-stats
brandonocasey May 27, 2021
44cda65
handle zero length media segments that did not append
brandonocasey May 27, 2021
36dfe49
Merge branch 'main' into feat/append-stats
brandonocasey May 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/master-playlist-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ const loaderStats = [
'mediaRequestsTimedout',
'mediaRequestsErrored',
'mediaTransferDuration',
'mediaBytesTransferred'
'mediaBytesTransferred',
'mediaAppends'
];
const sumLoaderStat = function(stat) {
return this.audioSegmentLoader_[stat] +
Expand Down Expand Up @@ -272,6 +273,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
// mediaRequestsErrored_
// mediaTransferDuration_
// mediaBytesTransferred_
// mediaAppends_
loaderStats.forEach((stat) => {
this[stat + '_'] = sumLoaderStat.bind(this, stat);
});
Expand All @@ -289,6 +291,23 @@ export class MasterPlaylistController extends videojs.EventTarget {
} else {
this.masterPlaylistLoader_.load();
}
const timeToCanPlayStart = Date.now();

this.timeToFirstFrame__ = 0;
this.appendsToFirstFrame__ = 0;

this.tech_.one('canplay', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once we 'canplay' a frame will be showing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently this is close to a "time to first byte" since if we are paused, the video may not get rendered immediately. playing or first timeupdate is potentially closer to a time to first rendered frame if we are autoplaying/not paused.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's this experimental API that can better represent the frame getting rendered https://wicg.github.io/video-rvfc/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switched it to loadeddata https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/loadeddata_event
as it specifically says: "The loadeddata event is fired when the frame at the current playback position of the media has finished loading; often the first frame."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that seems more like time to first byte rather than time to first frame, the latter implies that the frame has rendered which loadeddata doesn't guarantee it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked even deeper and tested on a few browsers, It seems like listening to loadeddata might be as close as we are going to get. For preload === 'none' we will have to start our timer on play but for all other values we should be able to start it on loadstart. Using those values gives somewhat consistent timeToFirstFrame for me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess what do you consider timeToFirstFrame? Is it the data has been loaded into the video element or the frame has rendered visibly to the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't have a good way to determine if the frame has been rendered visibly to the user (except for the experimental API), I think time to first frame should be when the video element reports that it has a buffered region containing currentTime. That might be a few ms before the frame is visible to the user, but I think it's the best cross browser solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I would probably want to rename the field then. We should also make sure to document exactly what it means so that we don't get confused in the future and have a place to reference it.
Also, does live have anything special for this beyond being equivalent to preload=none?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe timeToLoadedData and appendsToLoadedData? Live shouldn't be any different for VOD for these stats.

this.timeToFirstFrame__ = Date.now() - timeToCanPlayStart;
this.appendsToFirstFrame__ = this.mediaAppends_();
});
}

appendsToFirstFrame_() {
return this.appendsToFirstFrame__;
}
timeToFirstFrame_() {
return this.timeToFirstFrame__;

}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/segment-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ export default class SegmentLoader extends videojs.EventTarget {
this.mediaRequestsErrored = 0;
this.mediaTransferDuration = 0;
this.mediaSecondsLoaded = 0;
this.mediaAppends = 0;
}

/**
Expand Down Expand Up @@ -2988,6 +2989,8 @@ export default class SegmentLoader extends videojs.EventTarget {
// used for testing
this.trigger('appended');

this.mediaAppends++;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will show extra appends for:

  • segments with no data to be appended
  • sync requests that did not get appended

It will also under-report total number of appends, since we potentially append multiple times per segment.

Do we want to consider those cases and think about naming as "segmentAppends" since that may denote full segments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sync requests that do not get appended will return much earlier in the function. We could wrap mediaAppends increment in segmentInfo.hasAppendedData?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other problem with calling it segment appends is that we now append parts too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on sync requests

On wrapping and part appends, what is our use-case for this data? Do we want to know actual appends, or are we more concerned about requests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to know actual appends, we already have stats for requests.


if (!this.paused()) {
this.monitorBuffer_();
}
Expand Down
12 changes: 12 additions & 0 deletions src/videojs-http-streaming.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,18 @@ class VhsHandler extends Component {
get: () => this.masterPlaylistController_.mediaSecondsLoaded_() || 0,
enumerable: true
},
mediaAppends: {
get: () => this.masterPlaylistController_.mediaAppends_() || 0,
enumerable: true
},
appendsToFirstFrame: {
get: () => this.masterPlaylistController_.appendsToFirstFrame_() || 0,
enumerable: true
},
timeToFirstFrame: {
get: () => this.masterPlaylistController_.timeToFirstFrame_() || 0,
enumerable: true
},
buffered: {
get: () => timeRangesToArray(this.tech_.buffered()),
enumerable: true
Expand Down