fix: live dash segment changes should be considered a playlist update #1065
Conversation
src/playlist-loader.js
Outdated
| const newSidx = media.sidx; | ||
|
|
||
| // if sidx changed then the playlists are different. | ||
| if (oldSidx && newSidx && (oldSidx.offset !== newSidx.offset || oldSidx.length !== newSidx.length)) { |
There was a problem hiding this comment.
Now dash playlists can also be listed as changing in cases where:
- sidx does not match
- playlist segment uris don't match up
- playlist segment byterange uris don't match up.
src/playlist-loader.js
Outdated
| * null if the merge produced no change. | ||
| */ | ||
| export const updateMaster = (master, media) => { | ||
| export const updateMaster = (master, media, dash = false) => { |
There was a problem hiding this comment.
It might help separate things a bit if we move the DASH specific check into dash-playlist-loader. Maybe have a method updateAndCheckForChanges that wraps both updatePlaylist and didSegmentsChange or something like that.
Might also make it easier to test the function separately.
There was a problem hiding this comment.
Added a separate function isPlaylistUnchanged that is passed in as unchangedCheck to this function. Dash passes in it's own version that does all the dash specific things and also calls isPlaylistUnchanged at the top.
|
Testing with the US Live DASH stream, it's playing, though, after a bit it will bip bop and then buffer and then bip bop and then buffer, where-as before it would just eventually stall and buffer forever. |
|
Here's a quick screen recoding of how it looks for me: Screen.Recording.2021-02-10.at.15.40.16.mov |
|
It does seem to just stop working properly some times too. |
|
Seeking seems to fix it getting stuck, but then it gets stuck again. Really weird. |
|
Yeah there might be another issue here for that source. I think the behavior is better than it was though. |
brandonocasey
force-pushed
the
fix/live-dash
branch
from
d198f18 to
ba36f53
Compare
brandonocasey
force-pushed
the
fix/live-dash
branch
from
ba36f53 to
a8e6f8e
Compare
|
Do we want to get this merged in and then fix other live related stuff in subsequent PRs? |
|
I think we do, this fix is ready to go and fixes some streams right now |
|
|
||
| // if sidx changed then the playlists are different. | ||
| if (a.sidx && b.sidx && (a.sidx.offset !== b.sidx.offset || a.sidx.length !== b.sidx.length)) { | ||
| return false; |
There was a problem hiding this comment.
while I don't like returning booleans directly, this does seem like it's more readable than trying to convert all this into a single expression (even if it is possible)
| return true; | ||
| } | ||
|
|
||
| // check segments themselves |
There was a problem hiding this comment.
maybe worth checking a and b's segment's length first? If it changed, the manifest has updated, no?
There was a problem hiding this comment.
Ah, isPlaylistUnchanged does this check.
|
I don't have a dash byterange source, I was going off of an example I found where byterange was being used. |
Co-authored-by: Gary Katsevman <me@gkatsev.com>
|
Wouldn't it make sense to just fake the media sequence? We take the uri of the first segment in the new playlist, and find the matching segment in the old playlist. Use its index and the old playlist's media sequence to derive the new media sequence. If the segment isn't found, increase sequence by the number of segments to simulate falling behind the live window. Then As a test, I added to the start of Now the US Live DASH stream and other We'd need to determine the conditions for applying this delta. Perhaps at least Should fix #256 edit: opened PR #1092 |
Description
Currently dash content goes off of the
numberattribute for segments to determine the mediaSequence. This is problematic as the first segment is almost always listed as1. So in essence when we request the playlist again the media sequence will be the same, and likely the amount of segments will be the same, so we will determine that this playlist update didn't update anything at all.You can currently see this issue with the
Unified Streaming Live DASHsource. Eventually we will get into a state where the playlist isn't seen to be refreshing, even though the underlying segment uri's are changing every request.