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

Fix: HA & Demux - Audio Increment Not Respecting Position Diff-Threshold #301

Merged
merged 4 commits into from
Nov 1, 2023

Conversation

Nfrederiksen
Copy link
Collaborator

@Nfrederiksen Nfrederiksen commented Nov 1, 2023

When dealing with source demuxed vods which have video segment lengths longer than the audio counterparts, the CE is to increment the media sequence for the audio playhead position so that it is always further into the content than video, ie "fast forwarding" in a sense.
eg. If the current playhead position for video is 296 secs and audio is 291.84 secs, then audio would need to increment further until the audio position is >= video position.
However, there is a diff threshold set to 0.250s. This means that if the difference between the positions is below 0.250, then the CE is happy with the current positions. The difference should be insignificant for most players and the video segment should be able to be paired with the audio segment.
The issue is though that CE still increments by 2 even though it should be happy with one. This is caused by a mistake in a do-while loop.

This PR fixes this by refactoring the audio increment loop, replacing it with a new function and unit tests to validate its expected behaviors. The new function will also be used to calculate the increment value for subtitles, given that the subtitle delta positions are correct.

Copy link
Contributor

@birme birme left a comment

Choose a reason for hiding this comment

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

LGTM

@Nfrederiksen Nfrederiksen merged commit 5a52678 into master Nov 1, 2023
3 checks passed
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