Skip to content

Possible file corruption bug updating flac tag #442

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

Closed
milesegan opened this issue Aug 24, 2024 · 4 comments · Fixed by #446
Closed

Possible file corruption bug updating flac tag #442

milesegan opened this issue Aug 24, 2024 · 4 comments · Fixed by #446
Labels
bug Something isn't working
Milestone

Comments

@milesegan
Copy link

Reproducer

I set up a repro to demonstrate the issue here:

https://github.com/milesegan/lofty-flac-bug

Sorry about the big file and git lfs use. I tried to trim it down to a smaller example but couldn't get a smaller test file that reproduced the bug.

Summary

Adding a genre tag to an existing flac file renders the file unplayable. Performing the same operation via the metaflac command line tool does not exhibit the same issue.

Perhaps this is because the file has an empty GENRE= tag?

Expected behavior

No response

Assets

No response

@milesegan milesegan added the bug Something isn't working label Aug 24, 2024
@Serial-ATA
Copy link
Owner

Thanks for the detailed report as always! :)

I'll look into this tomorrow. Empty items shouldn't be an issue, they just get split normally and added as empty strings.

@Serial-ATA Serial-ATA added this to the 0.21.1 milestone Aug 24, 2024
@milesegan
Copy link
Author

milesegan commented Aug 24, 2024

Sorry about the big file. I wasn't able to trim it down to a smaller sample and still reproduce the issue. It might just be that the file itself is corrupted but it does play and it can be edited with metaflac.

@Serial-ATA
Copy link
Owner

Don't worry about the file size, any asset is better than none.

This was actually due to me mishandling PADDING blocks. I've never seen a file with padding anywhere but the end, so when writing it would make the assumption that it was at the end. That assumption means the Last-metadata-block flag wasn't getting set. Decoders need that flag to figure out where the audio data starts, and since it wasn't set they'd assume the stream was corrupted.

I made #445 to track padding usage, but for now I just have it discard PADDING blocks that aren't at the end of the header.

@milesegan
Copy link
Author

Thanks for another amazingly quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants