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

Support for low-latency media playlist #33

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Wkkkkk
Copy link

@Wkkkkk Wkkkkk commented Feb 10, 2025

This PR will try to add support for low-latency-related tags.

@Wkkkkk Wkkkkk self-assigned this Feb 10, 2025
Copy link
Collaborator

@tobbee tobbee left a comment

Choose a reason for hiding this comment

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

I think this is good in general, so I only have some smaller comments on the code.

There is one thing missing, though, and that is the circular buffer handling with a window. I think there should be a mechanism to automatically hide/remove the partial segments when they are too old.

It may also be good to have a method on the MediaPlaylist for adding a partial segment.

@Wkkkkk Wkkkkk requested a review from tobbee February 12, 2025 14:11
@Wkkkkk
Copy link
Author

Wkkkkk commented Feb 12, 2025

Hi @tobbee, this PR now includes all related changes to support low-latency streaming. It has been integrated into ll-hls-server and tested.

One major change was to rewrite the logic for 'winsize'. In the previous code, it is possible to decode a media playlist with more than 'winsize' number of segments. This kind of media playlist will only output the first 'winsize' number of segments when encoding.

Please feel free to leave your opinion.

@Wkkkkk Wkkkkk force-pushed the feat/support-low-latency branch from 34dd1bc to 8040ad0 Compare February 12, 2025 14:37
Copy link
Collaborator

@tobbee tobbee left a comment

Choose a reason for hiding this comment

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

Good work Kun. I have some small comments.

The major one is on how we should handle partial segments. I think we should keep them for the last three segments, which does not seem to be the case right now.

I also wonder why you want to increase the Go version to 1.22?

@@ -790,6 +882,22 @@ func (p *MediaPlaylist) Encode() *bytes.Buffer {
p.buf.WriteString(seg.ProgramDateTime.Format(DATETIME))
p.buf.WriteRune('\n')
}
// handle completed partial segments
if p.HasPartialSegments() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

My understanding was that we should typically keep the partial segments for the last three segments. To me, this looks as if you remove them for all full segments. I think that is too aggressive, but I may be wrong. What's your take on keeping them for three segments?

targetDurLocked bool // target duration is locked and cannot be changed
independentSegments bool // Global tag for EXT-X-INDEPENDENT-SEGMENTS
PartTargetDuration float64 // EXT-X-PART-INF:PART-TARGET
PartialSegments []*PartialSegment // List of partial segments in the playlist.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we make the PartialSegment part of the Segment instead?

I think that makes sense, although old segments should not keep that data.

@Wkkkkk
Copy link
Author

Wkkkkk commented Feb 13, 2025

To check if partial segments should be kept, I will try to compare its sequence number with the latest full segment number.

We are keeping partial segments in a separate list so to not add a new flag in Segment to indicate completed or not. It would also be difficult to parse #EXT-X-PART line as we would then have to append a uncompleted segment to segment list.

@Wkkkkk
Copy link
Author

Wkkkkk commented Feb 17, 2025

The last commit added support for "#EXT-X-SKIP:SKIPPED-SEGMENTS" tag.

HLS server should call SetSkip method for media playlists in response to '_HLS_skip' Delivery Directive.

@Wkkkkk
Copy link
Author

Wkkkkk commented Feb 21, 2025

  1. Limit the number of partial segments.
  2. Encode with skip as parameter.

@Wkkkkk Wkkkkk requested a review from tobbee February 25, 2025 12:23
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.

2 participants