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: add preliminary flac decoder #8

Closed
wants to merge 1 commit into from

Conversation

mewmew
Copy link
Member

@mewmew mewmew commented Aug 8, 2018

As noted in #3 (comment)
there are some bugs to be ironed out.

Updates #3.

@mewmew
Copy link
Member Author

mewmew commented Aug 8, 2018

@wsc1 Perhaps wait to merge this until we can iron out the known issue.

As noted in zikichombo#3 (comment)
there are some bugs to be ironed out.
@mewmew
Copy link
Member Author

mewmew commented Aug 18, 2018

@wsc1 FYI, the mewkiz/flac library now provides support for encoding FLAC streams (see mewkiz/flac#32). An encoder for zc could easily be added.

@wsc1
Copy link
Member

wsc1 commented Aug 18, 2018

This is very good news! It must have been a lot of work, with all the quantisation of lpc coefs and everything.

@mewmew
Copy link
Member Author

mewmew commented Aug 18, 2018

This is very good news! It must have been a lot of work, with all the quantisation of lpc coefs and everything.

Haha, yes. It would have been a lot of work, with LPC for sure! The PR adds basically WAV verbatim encoding for FLAC streams. There are four predictions methods used in FLAC

  • constant
  • verbatim
  • fixed: LPC with coefficients from a fixed set of polynomials
  • FIR: LPC with coefficients stored in 0th to 32nd order prediction polynomials.

Only verbatim is implemented now. Mainly to get the API right. Constant would be trivial to implement. LPC requires much more work, as you pointed out.

@mewmew
Copy link
Member Author

mewmew commented Aug 18, 2018

Only verbatim is implemented now. Mainly to get the API right. Constant would be trivial to implement. LPC requires much more work, as you pointed out.

Constant is implemented as of rev mewkiz/flac@a17e4ae.

@wsc1
Copy link
Member

wsc1 commented Aug 21, 2018

Does the decoder do all the modelling methods?

@mewmew
Copy link
Member Author

mewmew commented Aug 22, 2018

Does the decoder do all the modelling methods?

What do you mean by modelling methods?

@wsc1
Copy link
Member

wsc1 commented Aug 22, 2018

I mean prediction methods (lpc "fixed/FIR")?

@mewmew
Copy link
Member Author

mewmew commented Aug 22, 2018

The decoder handles all prediction methods (LPC "fixed/FIR"). The encoder however, only handles constant and verbatim (for now).

@wsc1
Copy link
Member

wsc1 commented Aug 22, 2018

Great news!

For the encoder, It may help us here at zc to hear about and/or help with how the FLAC lpc prediction is done. The zc/dsp/lpc module should be capable of doing the lpc part, although it's implementation is not the same as nor (yet) as fast as the FLAC one, so you probably wouldn't get exactly the same coefficiencts all the time. Not sure if you already have the modelling part of lpc in your decoder, as it's not strictly necessary for decoding. It would also help us to learn more about the subtleties around quantization of lpc coefficients, as a straightforward dumb quantisation won't work as well as what FLAC has, and there are publications around that topic of interest as well.

If you would be interested in sharing that journey, let us know when and how, fwiw, here or via mail or in another issue, would work for us.

@wsc1
Copy link
Member

wsc1 commented Aug 27, 2018

@mewmew

For info, I have added a function sound/cil.Compact which can make it easier to handler EOF.

@wsc1 wsc1 closed this Aug 27, 2018
@wsc1 wsc1 reopened this Aug 27, 2018
@wsc1
Copy link
Member

wsc1 commented Sep 2, 2018

What would you think of having a zc wrapper that imports zc and mewkiz flac in an "ext" repository under the zikichombo github organisation?

It would provide an easier reference than a PR for people to use and defer the question of preferred import direction.

@mewmew
Copy link
Member Author

mewmew commented Sep 2, 2018

What would you think of having a zc wrapper that imports zc and mewkiz flac in an "ext" repository under the zikichombo github organisation?

Sure, we can try it out.

@wsc1
Copy link
Member

wsc1 commented Sep 2, 2018

Great,

I'm new to using github to organise this kind of stuff...

I created a repo "github.com/zikichombo/ext" and a dedicated team under github.com/zikichombo
and sent you an invite to the team.

I think, and hope, it is likely to be used by others than us 2.

@wsc1
Copy link
Member

wsc1 commented Sep 3, 2018

This PR has been moved to zikichombo/ext#2 since it imports both zc and outside zc

@wsc1 wsc1 closed this Sep 3, 2018
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.

2 participants