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

FLAC decoder testbench #51

Closed
ktmf01 opened this issue Feb 19, 2022 · 7 comments
Closed

FLAC decoder testbench #51

ktmf01 opened this issue Feb 19, 2022 · 7 comments
Labels

Comments

@ktmf01
Copy link

ktmf01 commented Feb 19, 2022

I just found your FLAC decoding library, and tested it through flac2wav with this FLAC decoder testbench. I got quite a lot of messages like these

The flac library test cases do not yet include any audio files with xxx. If possible please consider contributing this audio sample to improve the reliability of the test cases.

For this I recommend the FLAC decoder testbench I just linked.

Besides the messages, flac2wav fails to decode the following files:

  • 16 - partition order 8 containing escaped partitions
  • 22 - 12 bit per sample
  • 32 - high resolution audio, partition order 8 containing escaped partitions
  • 37 - 20 bit per sample

Also, decoding of 23 - 8 bit per sample seems to be non-lossless.

I hope this information helps you to even further improve this library.

@mewmew
Copy link
Member

mewmew commented Feb 20, 2022

Thanks @ktmf01! That's definitely helpful. Having reference FLAC files will help us understand how these corner cases should be implemented.

Also, decoding of 23 - 8 bit per sample seems to be non-lossless.

Hmm, strange. I don't know where we would be introducing loss of the original PCM. Do you have any more info, e.g. what subframes were lossily decoded? (and what encoding was used for those subframes?)

Cheers,
Henry & Robin

@ktmf01
Copy link
Author

ktmf01 commented Feb 20, 2022

Hmm, strange. I don't know where we would be introducing loss of the original PCM. Do you have any more info, e.g. what subframes were lossily decoded? (and what encoding was used for those subframes?)

The whole file sound extremely distorted.
vergelijk

The upper two channels are original, the bottom two are the decoded WAV. The problem does not seem to involve the FLAC decoding itself, but instead the writing of the output to WAV. Programs seem to read the output wavefile as unsigned, while flac2wav seems to write signed output.

Now, obviously the writing to WAV part isn't the focus of your project, so you could just ignore this part.

edit: See page 59 - 60 of http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Docs/riffmci.pdf, WAV files with 8 bit-per-sample are stored with unsigned values, WAV files with more than 8 bit-per-sample are stored as signed values.

@mewmew
Copy link
Member

mewmew commented Feb 20, 2022

Programs seem to read the output wavefile as unsigned, while flac2wav seems to write signed output.

Interesting, this may indeed be the underlying issue.

Now, obviously the writing to WAV part isn't the focus of your project, so you could just ignore this part.

Yeah, you're right. The flac2wav is just an example tool to show how to use the FLAC library. I think we may ignore this, but would welcome a PR if there is an easy fix :)

As for the test cases, thanks once more! They will be a good reference for further testing the corner cases of the library. As for when those corner cases are implemented, I don't know :) My brother @karlek and I work on this project as a hobby so we do it when the mood strikes. We are really happy it has been useful to other people in the Go community, even if there are still corner cases left to handle.

Cheers,
Robin

@mewmew
Copy link
Member

mewmew commented Feb 20, 2022

Now, obviously the writing to WAV part isn't the focus of your project, so you could just ignore this part.

edit: See page 59 - 60 of http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Docs/riffmci.pdf, WAV files with 8 bit-per-sample are stored with unsigned values, WAV files with more than 8 bit-per-sample are stored as signed values.

@ktmf01, would you mind testing PR #52 and see if this resolves the encoding issue of the 8-bps test case?

@ktmf01
Copy link
Author

ktmf01 commented Feb 20, 2022

#52 indeed makes testbench file 23 decode correctly

mewmew added a commit that referenced this issue Feb 20, 2022
mewmew added a commit that referenced this issue Oct 24, 2023
Currently 7 test cases are broken, namely:
* subset/16 - ...flac
* subset/22 - ...flac
* subset/32 - ...flac
* subset/37 - ...flac
* subset/62 - ...flac
* subset/63 - ...flac
* subset/64 - ...flac

ref: #51
@mewmew mewmew added the bug label Oct 24, 2023
@mewmew
Copy link
Member

mewmew commented Oct 24, 2023

Issues related to FLAC stream decoding and audio sample decoding were tracked by #54 and #60, respectively, and are now resolved.

Before we decide to close this issue, we should try to assess whether there are any metadata blocks of the IETF test cases that mewkiz/flac/meta fails to parse. If so, new test cases can be added to mewkiz/flac/meta/meta_test.go.

  • check if mewkiz/flac/meta fails to parse any of the metadata of the IETF test case files.

@mewmew
Copy link
Member

mewmew commented Oct 25, 2023

To the best of my knowledge the IETF test cases are now passing for FLAC stream decoding, audio sample decoding and metadata decoding.

If any discrepancies are detected, feel free to re-open this issue or open a new one.

Thanks @ktmf01 for driving this validation effort. You are part of something incredible <3

@mewmew mewmew closed this as completed Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants