Skip to content

Conversation

@Serial-ATA
Copy link
Owner

This allows us to sift through junk bytes to find what we need.

Discovered through discussion #219.

Determined(FileType),
/// The stream starts with an ID3v2 tag
MaybePrecededById3(u32),
/// The stream starts with junk zero bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the zero bytes at the beginning called junk while at the end they are denoted as padding? Would it be suitable to consider both as padding to avoid introducing a new terminology?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or might junk also contain non-zero bytes? Then this comment would be imprecise.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It would be junk in either case. Once it's out of the context of a tag, anything that was once padding is now garbage data. Where is it referred to as padding? It should be updated.

Or might junk also contain non-zero bytes? Then this comment would be imprecise.

It's currently only zeroes, but it could be changed to be anything. It's only a matter of time before some files come along with some other types of junk, as seen in the case of #28.

@sandersantema
Copy link

This fixes the issue for me for all the tracks I had which weren't working, so afaik it is fixed 🎉 Thanks a lot.

@sandersantema
Copy link

Just to confirm: this is expected to work without changing any options or setting the ParseOptions correct? That's the behavior I'm seeing in any case.

@Serial-ATA
Copy link
Owner Author

Yep, ParseOptions::max_junk_bytes defaults to 1024.

@Serial-ATA Serial-ATA merged commit 9e8c0a9 into main Jul 8, 2023
@Serial-ATA Serial-ATA deleted the probe-junk branch July 8, 2023 16:57
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.

4 participants