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.

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

Handle #EXT-X-ENDLIST appended to live playlist without new segment #5778

Merged
merged 3 commits into from
Aug 31, 2023

Conversation

robwalch
Copy link
Collaborator

This PR will...

Treat LevelDetails as updated when #EXT-X-ENDLIST is appended to live playlist without new segment. Mark the corresponding segment in the previous details object with endList = true so that the fragment-tracker can add it to endListFragments.

Why is this Pull Request needed?

endListFragments is used to determine that everything from the playhead to the end of the playlist has been buffered so that the MediaSource and SourceBuffer(s) can be marked as ended.

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Fixes #5777

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@robwalch robwalch added this to the 1.5.0 milestone Aug 29, 2023
@robwalch
Copy link
Collaborator Author

cc @MosheMaorKaltura

Copy link
Collaborator

@OrenMe OrenMe left a comment

Choose a reason for hiding this comment

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

LGTM! We also have clear repro so will be tested before to make sure it solves the issue

@semarche-kaltura
Copy link

semarche-kaltura commented Aug 29, 2023

The issue #5777 still reproduces with bugfix/live-append-only-endlist branch.
Short investigation shows that when parseLevelPlaylist handles ENDLIST and oldFrag.endList = true; method detectPartialFragments doesn't executes (endListFragments object keeps empty), so isEndListAppended always returns false.

@robwalch
Copy link
Collaborator Author

The issue #5777 still reproduces with bugfix/live-append-only-endlist branch.
Short investigation shows that when parseLevelPlaylist handles ENDLIST and oldFrag.endList = true; method detectPartialFragments doesn't executes (endListFragments object keeps empty), so isEndListAppended always returns false.

If the last segment is appended before the ENDLIST is published and loaded that is definitely possible.

@robwalch
Copy link
Collaborator Author

robwalch commented Aug 29, 2023

@semarche-kaltura, 89c648f 9c9e79e will force detectPartialFragments when the playlist updates with ENDLIST now.

It might be worth noting that when the stream does end, because it has a sliding window, it cannot behave as a VOD in the current session since there are no segments available at 0.

Copy link

@semarche-kaltura semarche-kaltura left a comment

Choose a reason for hiding this comment

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

With 89c648f playback works as expected! 👍

@semarche-kaltura
Copy link

@robwalch we would want have the changes as "patch" but I don't have permissions for open a PR against the "patch/v1.4.x" branch.

@robwalch
Copy link
Collaborator Author

@robwalch we would want have the changes as "patch" but I don't have permissions for open a PR against the "patch/v1.4.x" branch.

You can make a commit off of a fork of that branch and then open a PR. If GitHub then does not let you change the target branch of your PR, @OrenMe or I can.

@robwalch robwalch force-pushed the bugfix/live-append-only-endlist branch from 89c648f to 108315c Compare August 30, 2023 23:59
@semarche-kaltura
Copy link

semarche-kaltura commented Aug 31, 2023

@robwalch I've opened PR from forked repo (base "patch/v1.4.x" branch + cherrypicked commits from "bugfix/live-append-only-endlist" branch) into "patch/v1.4.x" branch, check if everything made correct. Thanks.

@robwalch robwalch merged commit 3529b75 into master Aug 31, 2023
@robwalch robwalch deleted the bugfix/live-append-only-endlist branch August 31, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENDED event is not raised on live stream, even though #EXT-X-ENDLIST arrived and no more manifests
3 participants