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

Last call on FLAC spec #53

Closed
ktmf01 opened this issue Oct 1, 2022 · 3 comments
Closed

Last call on FLAC spec #53

ktmf01 opened this issue Oct 1, 2022 · 3 comments
Labels

Comments

@ktmf01
Copy link

ktmf01 commented Oct 1, 2022

Sorry for bothering you this way, I really hope you don't mind.

To implement FLAC decoding in your flac implementation, you might have used the FLAC format specification on the FLAC website. If so, you probably needed to dive into some actual code (libFLAC or ffmpeg) to understand the finer points. An IETF working group has tried to improve this document, trying to include those finer points, and is on the verge of turning it in for final checks and publication as an RFC. A so-called working group last call has been issued, which lasts 2 weeks.

As you've had first-hand experience implementing FLAC from scratch, you probably know best what implementation details needed extra attention. It would be very valuable to have your feedback on the FLAC format for this document.

I hope you can provide some insights.

The latest draft can be found here https://datatracker.ietf.org/doc/draft-ietf-cellar-flac/

@mewmew
Copy link
Member

mewmew commented Oct 4, 2022

Hej @ktmf01,

Thanks for working on this!

My brother Henry and I had a lot of fun implementing the FLAC decoder library in Go. The xiph.org FLAC spec was quite well written with regards to enums, bit representations, steam block types, metadata, etc.

Our main concern with the xiph.org FLAC spec was the missing information on how samples were encoded using LPC, and how residuals were encoded using Rice coding. For Rice coding we had to refer to other sources than the xiph.org FLAC spec to be able to implement our decoder.

Having pseudo-code that handles Rice decoding be part of the spec would have been really useful.

Other than that, the FLAC format has proven very stable, and the xiph.org spec was enough to implement most parts of the decoder.

It is true that there have been issues in our decoder, as have been identified and fixed by members of the open source community : )

For instance, Jonathan MacMillan fixed two subframe decoding issues related to integer truncation of samples during LPC multiplication and a typo related to Rice code parameter size (#7); and Sergey Didyk fixed decoding of the "fixed block size" boolean (#9).

We were also really thrilled to have the library be fuzzed by Patrick Mézard (in #10), which also helped uncover a range of stability issues.

For a long time we were unable to find test cases that would help exercise parts of our code. So, we added "debug messages" that would encourage users of the library to submit test cases for these corner cases; such as the 16kHz test cases contributed by Chewxy.

We're excited to see that the test cases released by the IETF also helped cover a lot of these corner cases. Thanks for publishing them!

A quick test also uncovered decoding issues for at least 3 of the 64 FLAC test files of the ietf-wg-cellar/flac-test-files repository (see #54). As we get time to do more thorough tests, these files could also help uncover the more fragile areas of metadata decoding.

So, to sum up.

Thank you and the rest of the IETF for working on establishing a stand-alone FLAC spec that is self-contained and sufficient for implementing FLAC decoders. Our primary wish is for the LPC and Rice coding parts to be detailed with rigor, and if possible that pseudo-code was added for these more intricate sections of the FLAC spec.

Wish you all the best and a most wonderful Autumn.

With kindness,
Robin & Henry Eklind

@ktmf01
Copy link
Author

ktmf01 commented Oct 5, 2022

Hi @mewmew,

Our main concern with the xiph.org FLAC spec was the missing information on how samples were encoded using LPC, and how residuals were encoded using Rice coding. For Rice coding we had to refer to other sources than the xiph.org FLAC spec to be able to implement our decoder.

Having pseudo-code that handles Rice decoding be part of the spec would have been really useful.

Those concerns have been addressed. The document now really tries to explain what is happening, instead of just summing things up. I have chosen not to use pseudocode, but instead tried to explain as much as possible in text. There is an informational appendix with examples where FLAC files are decoded 'by hand', which I think serves the same purpose.

Thanks for taking the time to reply.

@mewmew
Copy link
Member

mewmew commented Oct 5, 2022

There is an informational appendix with examples where FLAC files are decoded 'by hand', which I think serves the same purpose.

Yeah, the appendix with the examples was incredibly useful! I wish all documentation was written like this : )

Cheers,
Robin

@mewmew mewmew added the meta label Oct 5, 2022
@ktmf01 ktmf01 closed this as completed Oct 29, 2022
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