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

Use eof_action=pass when overlaying subtitles #10710

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

jkhsjdhjs
Copy link
Contributor

The previous behavior using eof_action=endall and shortest=1 would end the video stream if one of the input stream (video, subtitle) ends. In some cases the duration of the overlayed subtitles is shorter than the video stream, causing the output to end when the subtitles end and dropping the remaining video stream.

This commit changes this behavior so eof_action=pass is used instead, which continues passing the video stream through even if the subtitles end earlier [1]. shortest=1 is also removed, as this option implies eof_action=endall.

If the subtitle stream has a higher duration than the video stream, the output will also end with the video stream without shortest=1, as the video stream is the primary input to the overlay filter.

Fix #10698

[1] https://ffmpeg.org/ffmpeg-filters.html#Options-for-filters-with-several-inputs-_0028framesync_0029

Copy link
Member

@nyanmisaka nyanmisaka left a comment

Choose a reason for hiding this comment

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

overlay_vulkan should also apply the same change.

overlayFilters.Add("overlay_vulkan=eof_action=endall:shortest=1:repeatlast=0");

@jkhsjdhjs jkhsjdhjs force-pushed the fix/subtitle_overlay_eof_action branch from 8252731 to bcc376a Compare December 12, 2023 22:40
@jkhsjdhjs
Copy link
Contributor Author

overlay_vulkan should also apply the same change.

overlayFilters.Add("overlay_vulkan=eof_action=endall:shortest=1:repeatlast=0");

Good catch! I initially did the change on the release-10.8.z branch, which doesn't have the overlay_vulkan line. I just fixed it.

@JPVenson JPVenson added enhancement Improving an existing function, or small fixes good first issue Good for newcomers backend Related to the server backend labels Dec 12, 2023
@nyanmisaka
Copy link
Member

Good catch! I initially did the change on the release-10.8.z branch, which doesn't have the overlay_vulkan line. I just fixed it.

Found another one. Please remove the :shortest=1 in this line.

overlayFilters.Add("overlay=eof_action=pass:shortest=1:repeatlast=0");

The previous behavior using `eof_action=endall` and `shortest=1` would
end the video stream if one of the input stream (video, subtitle) ends.
In some cases the duration of the overlayed subtitles is shorter than the
video stream, causing the output to end when the subtitles end and dropping
the remaining video stream.

This commit changes this behavior so `eof_action=pass` is used instead,
which continues passing the video stream through even if the subtitles end
earlier [1]. `shortest=1` is also removed, as this option implies
`eof_action=endall`.

If the subtitle stream has a higher duration than the video stream, the output
will also end with the video stream without `shortest=1`, as the video stream
is the primary input to the `overlay` filter.

Fix jellyfin#10698

[1] https://ffmpeg.org/ffmpeg-filters.html#Options-for-filters-with-several-inputs-_0028framesync_0029
@jkhsjdhjs jkhsjdhjs force-pushed the fix/subtitle_overlay_eof_action branch from bcc376a to 547d97d Compare December 13, 2023 12:11
@jkhsjdhjs
Copy link
Contributor Author

Found another one. Please remove the :shortest=1 in this line.

overlayFilters.Add("overlay=eof_action=pass:shortest=1:repeatlast=0");

Done. I found two other occurances of the same line and removed the shortest=1 there as well.

@cvium cvium merged commit 676464a into jellyfin:master Dec 13, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to the server backend enhancement Improving an existing function, or small fixes good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue]: Use eof_action=pass when overlaying subtitles
5 participants