Processing segment reachable even after playlist update removes it

[gesinger]

Description

In live scenarios, a refresh may render a processing segment unreachable. This attaches the segment to the segmentInfo object so the segment will always be around.

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Unit Tests updated or fixed
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[imbcmdth]

When a playlist is refreshed, the segments in the new playlist are copies of the old segments. That means that if playlist is refreshed between segment request and xhr completing, we will annotate the old segment object at:

https://github.com/gesinger/videojs-contrib-hls/blob/b426cc22fe27de3d96a53d5586c0ab3a451d3ad1/src/sync-controller.js#L310-L314

[gesinger]

Good call @imbcmdth . Here are a couple approaches (I think (1) is better, but there are definitely alternatives from these as well):

1. When we update segment loader's playlist with a refresh and change the media index of segmentInfo, we also change the segment reference. We'd have to check that the segment exists on the new playlist (media index did not turn negative). If it doesn't exist, we simply leave the old segment reference (it'll save info onto a segment that is missing from the playlist, but no harm will be done). This is the simplest change from the current code.

2. Instead of saving the segment reference on segmentInfo, we continue to use mediaIndex and the playlist reference but save old segments that are not officially part of the playlist anymore for the case where they fell off. This may lead to misleading playlist objects, as we will save information that technically no longer exists in the playlist.

[mjneil]

I think it would be good to also check loader.pendingSegment_.segment.uri is what you expect whenver checking loader.pendingSegment_.uri to to make sure the new segment object we are attaching to segmentInfo is the correct reference

[mjneil]

Why do we only do these adjustments for non-sync requests. It probably doesn't matter if we don't save the info on an outdated segment object when its a sync-request because we don't append it and if we do need to append it in the future, we are going to have to re-request/probe/sync it anyway. However, I think it probably doesn't hurt to make sure the timing info we get from the probe of the sync request to be put onto the segment object. It might help make other calculations that fallback to target duration when start and end times are unavailable be more accurate

[imbcmdth]

LFGTM