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

Do not adjust initPTS without accurate playlist time offset #6922

Merged
merged 4 commits into from
Dec 21, 2024

Conversation

robwalch
Copy link
Collaborator

@robwalch robwalch commented Dec 20, 2024

This PR will...

  • Do not adjust initPTS without accurate playlist time offset (when live playlist is out of sync).
  • Do not re-emit AUDIO_TRACK_LOADED for track sync as it reruns playlist merge logic.
  • Keep audio-stream-controller startPosition within playlist bounds.
  • Fix audio-only main <-> track switching buffer flushing issues
  • Do not treat live playlists as expired after three "misses" (static responses -> archived live / load segments anyway)

Why is this Pull Request needed?

Adjusting initPTS should only be performed for contiguous operations or on switch when expected timestamps differ enough from playlist times that the only way forward is to remap them (these are malformed HLS but we support them since #5235 and #5471).

In this case, when live playlist updates are unable to be delivered frequently enough to stay in sync, media timestamps are used to realign them, but this remapping of initPTS prevented that adding permanent misalignment.

Re-emitting AUDIO_TRACK_LOADED when cachedTrackLoadedData caused mergeDetails to be called on the audio playlist details a second time shifting it forward.

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

accurateTimeOffset is false because the playlist time should not be trusted for segments from live playlists which are being loaded out of sequence. In this state, initPTS should not be readjusted.

Resolves issues:

Resolves #6920

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

@kakao-wise-kim
Copy link

@robwalch
hi. robwalch
Testing with the this submitted PR code appears to have resolved the audio synchronization issue.
However, the audio segment download behavior seems suspicious.
As shown in the attached image, it first fetches an earlier audio segment.
For example, assuming the current segment index is 165666, it downloads audio index 165667 first, and then proceeds to download video and audio index 165666.

image image

@kakao-wise-kim
Copy link

I attached Chrome Media Event and Message

image image

@robwalch
Copy link
Collaborator Author

Testing with the this submitted PR code appears to have resolved the audio synchronization issue.

👍

However, the audio segment download behavior seems suspicious.

I attached Chrome Media Event and Message

This has nothing to do with Chrome's decoder. If playlists become desynced because of insufficient bandwidth, you can expect segments to be loaded out of sequence. Debug logs would include information about hls.js behavior in that case.

… merge logic

Keep audio-stream-controller startPosition within playlist bounds
@robwalch
Copy link
Collaborator Author

However, the audio segment download behavior seems suspicious.

@kakao-wise-kim please give it another try with changes 5ca9822. The audio playlist was drifting ahead. Hopefully selection is less suspicious now. 🕺 💃

@kakao-wise-kim
Copy link

kakao-wise-kim commented Dec 20, 2024

@robwalch
The issue of audio drifting forward slightly seems to have been alleviated, but it hasn’t completely disappeared.
Since the video also follows closely, it seems to be acceptable to a certain extent.

@kakao-wise-kim
Copy link

Occasionally, the audio is received, but instead of the video at the same timeline, an earlier video is received.
This sometimes results in playback with 2 to 3 previous audio-video segments.

image

@robwalch
Copy link
Collaborator Author

Occasionally, the audio is received, but instead of the video at the same timeline, an earlier video is received.

That is unrelated to these changes. Please file a new issue.

@kakao-wise-kim
Copy link

Occasionally, the audio is received, but instead of the video at the same timeline, an earlier video is received.

That is unrelated to these changes. Please file a new issue.

I think it’s problematic if the playback position moves forward and skips downloaded segments. However, since the current system waits for the download to complete before starting playback, it does not skip the switched segments. Therefore, I do not consider this issue a problem.

If the system is modified to download starting from the earliest available index, it would be an improvement in terms of optimization.

Fix buffer flushing when switching between audio-only from main and alt (variant and media playlists)
Add additional test coverage
@robwalch robwalch force-pushed the bugfix/live-av-desync-init-pts branch from f6baef4 to 17a49cf Compare December 21, 2024 00:36
Improve audio-stream-switched logging
@robwalch robwalch merged commit e811aba into master Dec 21, 2024
16 checks passed
@robwalch robwalch deleted the bugfix/live-av-desync-init-pts branch December 21, 2024 20:38
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.

Timeout Occurs and Playback Reverts to an Earlier Position When Manipulating Network Bandwidth
2 participants