fix: Fix playRangeEnd for certain content #4068
Conversation
joeyparrish
force-pushed
the
fix-play-range-end
branch
from
21e7a26
to
2738bfb
Compare
For content where the manifest parser uses the notifySegments API (DASH SegmentBase/SegmentList/SegmentTimeline, or HLS, but not DASH SegmentTemplate+duration), the Player seekRangeEnd configuration was not being honored correctly. This fixes the issue in PresentationTimeline and adds a regression test to our playRangeStart/playRangeEnds tests. Fixed shaka-project#4026
joeyparrish
force-pushed
the
fix-play-range-end
branch
from
2738bfb
to
8c85bd3
Compare
| @@ -361,6 +361,13 @@ shaka.media.PresentationTimeline = class { | |||
| getSegmentAvailabilityEnd() { | |||
There was a problem hiding this comment.
The JSDoc for the return value, above, says "Aways returns the presentation's duration for video-on-demand." That seems to be pretty out-of-date, now.
lib/media/presentation_timeline.js
Outdated
| @@ -361,6 +361,13 @@ shaka.media.PresentationTimeline = class { | |||
| getSegmentAvailabilityEnd() { | |||
| if (!this.isLive() && !this.isInProgress()) { | |||
| // It's a static manifest (can also be a dynamic->static conversion) | |||
| if (this.maxSegmentEndTime_ && this.duration_) { | |||
There was a problem hiding this comment.
Do we care about the possibility of duration_ or maxSegmentEndTime_ being 0 here?
If not, you don't need to check if duration_ is truthy, because it's a non-nullable number and it initializes to Infinity.
| player.configure({playRangeStart: 5, playRangeEnd: 10}); | ||
| const timeline = new shaka.media.PresentationTimeline(300, 0); | ||
| timeline.setStatic(true); | ||
| timeline.setDuration(300); |
There was a problem hiding this comment.
It's a bit confusing how in this test, playRangeEnd corresponds with duration_ inside the timeline, and the setDuration method call corresponds with maxSegmentEndTime_ in the timeline. Feels like the two things named duration should match up.
That's out of the scope of this CL though.
There was a problem hiding this comment.
Yeah, it's weird. What's really happening is that stream.useSegmentTemplate refers to timeline's duration to decide how many references to generate. Then player.load applies the config to the timeline, and sets the duration lower.
What would be less confusing, I think, is if Timeline had a separate field for the playRangeEnd setting. But that would also require a lot more ifs or mins in Timeline, which could be confusing in a different way.
- Update jsdoc in Timeline - reorganize and add comments to tests to clarify
For content where the manifest parser uses the notifySegments API (DASH SegmentBase/SegmentList/SegmentTimeline, or HLS, but not DASH SegmentTemplate+duration), the Player seekRangeEnd configuration was not being honored correctly. This fixes the issue in PresentationTimeline and adds a regression test to our playRangeStart/playRangeEnds tests. Fixed #4026
For content where the manifest parser uses the notifySegments API (DASH SegmentBase/SegmentList/SegmentTimeline, or HLS, but not DASH SegmentTemplate+duration), the Player seekRangeEnd configuration was not being honored correctly. This fixes the issue in PresentationTimeline and adds a regression test to our playRangeStart/playRangeEnds tests. Fixed #4026
For content where the manifest parser uses the notifySegments API (DASH SegmentBase/SegmentList/SegmentTimeline, or HLS, but not DASH SegmentTemplate+duration), the Player seekRangeEnd configuration was not being honored correctly. This fixes the issue in PresentationTimeline and adds a regression test to our playRangeStart/playRangeEnds tests. Fixed #4026
github-actions
bot
added
the
status: archived
For content where the manifest parser uses the notifySegments API
(DASH SegmentBase/SegmentList/SegmentTimeline, or HLS, but not DASH
SegmentTemplate+duration), the Player seekRangeEnd configuration was
not being honored correctly.
This fixes the issue in PresentationTimeline and adds a regression
test to our playRangeStart/playRangeEnds tests.
Fixed #4026