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

#7109: Use segment or audio samples duration in case of single sample video chunk #7112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mstyura
Copy link
Contributor

@mstyura mstyura commented Mar 20, 2025

This PR will...

Change the computation of duration of MP4 video sample when transmuxed from MPEG-TS fragment with single video sample. The duration of segment reported via #EXTINF tag will be used as duration of single video sample or if this duration now available - the length of audio samples will be used.

Why is this Pull Request needed?

Currently playback of HLS streams with TS fragments stuck in case of fragment has single video sample.

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

The issue has been encountered before and reported as #4783 and supposedly fixed by #4794.
It also seems like this is kind of corner case for TS -> MP4 transmuxing which also encountered in Shaka Player.

Resolves issues:

#7109

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

PS: This is probably just one approach to resolve problem and it might be not fully correct. The feedback from maintainers much appreciated.

const audioLengthBasedSampleDuration = audioTrackLength
? Math.round(audioTrackLength * track.inputTimeScale)
: track.inputTimeScale;
const fragmentLengthOrAudioLengthBasedSampleDuration = fragmentDuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be incorrect to use fragment duration in the case where an LL-HLS part is being processed, or a partial segment is being processed (with progressive: true config option).

We'll need to test this against a variety of media with and without that config to confirm it is safe.

This makes sense to include in v1.7 which will focus on I-frame support since that will involve supporting single sample video segments that ideally span full segment durations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be incorrect to use fragment duration in the case where an LL-HLS part is being processed, or a partial segment is being processed (with progressive: true config option).

I agree, I've tried to "workaround" this by not passing duration of incomplete chunks, but not sure all the cases handled correctly. For some reason I assumed it is not that important because I thought LLHLS is defined only over mp4 chunks, but right now I can't find this in latest rfc.

This makes sense to include in v1.7 which will focus on I-frame support since that will involve supporting single sample video segments that ideally span full segment durations.

I have nothing against this decision, the PR is just an example of solving the issue for the example stream. I hope during development of version 1.7 more robust solution will be considered and hopefully implemented.

@robwalch robwalch added this to the 1.7.0 milestone Mar 22, 2025
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