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

avformat/assenc: do not copy null terminator #509

Merged
merged 2 commits into from
Dec 14, 2024
Merged

avformat/assenc: do not copy null terminator #509

merged 2 commits into from
Dec 14, 2024

Conversation

gnattu
Copy link
Member

@gnattu gnattu commented Nov 26, 2024

The par->extradata buffer filled from some matroska files may be null terminated, and use ffio_write_lines using the full buffer length will copy this null character into the output files. This results in a file in which there is a null terminator after the header, but preceeding the actual content of the subtitle file. Treat this buffer as a string and write line with the text length of this buffer to skip the null character.

Regression from 7bf1b9b

Fixes #506

Changes

Issues

@gnattu gnattu requested a review from a team November 26, 2024 05:47
debian/patches/series Outdated Show resolved Hide resolved
@nyanmisaka
Copy link
Member

If this is the cause, it would be nice to have the upstream add a sample to their fate-tests.

@gnattu
Copy link
Member Author

gnattu commented Nov 26, 2024

The problem is I'm unable to reproduce with any of the files and handbrake combinations, the sample file I used for testing is binary edited to contain the null char. The sample file in the original ticket is no longer downloadable.

The `par->extradata` buffer filled from some matroska files may be
null terminated, and use `ffio_write_lines` using the full buffer
length will copy this null character into the output files. This
results in a file in which there is a null terminator after the
header, but preceeding the actual content of the subtitle file.
Treat this buffer as a string and write line with the text length of this buffer to skip the null character.

Regression from 7bf1b9b
@nyanmisaka
Copy link
Member

The problem is I'm unable to reproduce with any of the files and handbrake combinations, the sample file I used for testing is binary edited to contain the null char. The sample file in the original ticket is no longer downloadable.

Exactly. Using those short links is terrible for archiving.
I also tried searching locally but it seems I've deleted it because it's been so long.

@gnattu gnattu merged commit b7d81fc into jellyfin Dec 14, 2024
27 checks passed
@gnattu gnattu deleted the fix-ass-null branch December 14, 2024 07:55
@gnattu
Copy link
Member Author

gnattu commented Dec 14, 2024

Merging as confirmed to fix the issue

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.

ASS Sub BUG
3 participants