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

Valid files(checked by FFprobe) aren't properly parsed in Symphonia #124

Open
qarmin opened this issue May 9, 2022 · 7 comments
Open

Valid files(checked by FFprobe) aren't properly parsed in Symphonia #124

qarmin opened this issue May 9, 2022 · 7 comments
Milestone

Comments

@torokati44
Copy link

I don't know if their license allows looking at it, or taking inspiration from it, but FFmpeg does a more elaborate check during MP3 frame header synchronization; looking ahead in the buffer and scoring any candidates, then picking the one that looks best: https://github.com/FFmpeg/FFmpeg/blob/38238b604fb59ebaafe93dcf72e544cb33c4ac70/libavformat/mp3dec.c#L499-L547=

@qarmin qarmin changed the title Valid files(checked by FFprobe) doesn't properly parse in Symphonia Valid files(checked by FFprobe) aren't properly parsed in Symphonia May 10, 2022
@pdeljanov
Copy link
Owner

I didn't try them all, but for all the MP3 files I tried, the issue seems to be within the ID3v2 parser being too strict. It's encountering a tag with size == 0 which is not allowed by the spec. and returning an error. That error then gets propagated and causes the entire stream to be rejected. With a bit of work, the parser can be made more lenient to allow this case.

The WAV files seems to have a similar issue, but I'd need to look more deeply.

As for the AC3 file, Symphonia does not support AC3. :)


I don't think gaining mild inspiration is problematic, but FFMpeg is LGPL licensed and Symphonia is MPL licensed so copying isn't allowed since our license has fewer restrictions. Either way, for these issues it doesn't seem to be the packet parsing that's the issue.

@torokati44
Copy link

Either way, for these issues it doesn't seem to be the packet parsing that's the issue.

Okay, that's good! I just thought that it is a regression in 8f4aaed as it was explicitly mentioned. Sorry for the confusion.

pdeljanov added a commit that referenced this issue May 30, 2022
pdeljanov added a commit that referenced this issue May 30, 2022
@pdeljanov
Copy link
Owner

The broken MP3s should be fixed with the last two commits.

@pdeljanov
Copy link
Owner

Regarding the wave files:

  • sample.wav and idw_testsound.wav are ADPCM Wave files and are unsupported currently. ADPCM support #41 tracks ADPCM support.
  • jetpack1.wav is technically an invalid encoding, but Symphonia should probably handle it.

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

No branches or pull requests

3 participants