Fix video corruption on rendition switches in IE11 Win 8.1+ and Edge #1259
Conversation
mjneil
changed the title
mjneil
force-pushed
the
fix/green-green
branch
2 times, most recently
from
52c273b
to
7274209
Compare
src/segment-loader.js
Outdated
| @@ -1120,6 +1118,15 @@ export default class SegmentLoader extends videojs.EventTarget { | |||
| this.trigger('timestampoffset'); | |||
| } | |||
|
|
|||
| if (timingInfo && timingInfo.hasMapping) { | |||
There was a problem hiding this comment.
Since the calculateSegmentTimeMapping_ function is simple enough, maybe it would be better to change the function to get the segment time mapping and call it again here instead of including the hasMapping property. But either way is fine.
There was a problem hiding this comment.
made a function mappingForTimeline
| @@ -612,6 +612,13 @@ export class MasterPlaylistController extends videojs.EventTarget { | |||
| this.tech_.trigger('hls-reset'); | |||
| }); | |||
|
|
|||
| this.mainSegmentLoader_.on('segmenttimemapping', (event) => { | |||
There was a problem hiding this comment.
I know we discussed it, but I'm still uneasy about using the event to communicate with videojs-contrib-media-sources. Although it's better not to add non-standard APIs to videojs-contrib-media-sources, making an event is essentially creating an API for it anyway, and makes the code harder to follow. Although it does help in cases where we don't want to fail on native media sources, I'm thinking that it might be better for us to have a specific method in source updater and contrib-media-sources to accept the data, just to make things easier to follow. Eventually contrib-media-sources will go away, removing the issue.
I do understand the arguments for using the event though (in that we aren't adding a "supported" non standard API to videojs-contrib-media-sources). If we do go with the event route, we should at least add some comments here and in videojs-contrib-media-sources to more easily understand how it is being passed between the two projects.
There was a problem hiding this comment.
I do understand that the event is more difficult to follow than a direct api, but since we plan on moving away from contrib-media-sources (sooner the better eh) and we already use this event approach else where, I added comments so we can save time on testing a refactor. I've also added comments in the contrib-media-sources pr
mjneil
force-pushed
the
fix/green-green
branch
from
a5f6841
to
eda4e01
Compare
Description
Some browsers (cough IE11 and Edge cough) have a more simple, though still spec compliant, implementation of MediaSourceExtensions compared to other browsers. The main issue with this simpler implementation is that MSE will drop data until the next key frame that comes after the time already sent to the decoder. What this means is you cannot append content that overlaps with the playhead (something videojs-contrib-hls relies on for faster mid-segment quality switching) without the risk of visual corruption since the frames needed to decode the new content at the playhead are dropped before being sent to the decoder.
REQUIRES THE FOLLOWING FOR FULL SUPPORT
videojs/videojs-contrib-media-sources#164
videojs/mux.js#162
Specific Changes proposed
hls-segment-time-mappingis triggered on thetechforHtmlMediaSourcefrom videojs-contrib-media-sources to listen to so that theVirtualSourceBuffercan properly convert display time to TS time and trim segment appends accordingly.