mewkiz/flac

Re-Implement the flac.Encode(f, stream) function

spbkaizo opened this issue · 5 comments

Hi, It's great that you're attempting to implement a new encoder API, but after some testing with it I've not got a path forward with existing files, which are using a non-implemented prediction codec:

ERROR: github.com/mewkiz/flac.encodeSubframe (encode_subframe.go:38): error: support for prediction method 3 not yet implemented

Previously, I could use flac.Encode(...) to achieve simple metadata editing, which works on those files but that was removed in favour of the new Encoder API.

Looking for a suggestion as to how best to proceed - should I attempt to re-implement the flac.Encode function within my own code, or is there something I'm missing when trying to change just the metadata, and not the samples.

Hi @spbkaizo,

Hi, It's great that you're attempting to implement a new encoder API, but after some testing with it I've not got a path forward with existing files, which are using a non-implemented prediction codec:

ERROR: github.com/mewkiz/flac.encodeSubframe (encode_subframe.go:38): error: support for prediction method 3 not yet implemented

Yeah. This is to be expected, as support for LPC encoded samples is yet to be implemented. In essence only verbatim encoding of audio samples is currently implemented.

For background, refer to issue #35 and #37.

Previously, I could use flac.Encode(...) to achieve simple metadata editing, which works on those files but that was removed in favour of the new Encoder API.

The previous encoding API was essentially a hot-fix to allow for metadata encoding (for background, see issue #14), but it didn't try to re-encode audio samples. The new API is intended to be generic and support also re-encoding of audio samples (see #15). However, as stated above, support for LPC encoding of audio samples has not yet been implemented.

Looking for a suggestion as to how best to proceed - should I attempt to re-implement the flac.Encode function within my own code, or is there something I'm missing when trying to change just the metadata, and not the samples.

I'd suggest that you either roll your own metadata encoding API (feel free to copy the metadata encoding API of mewkiz/flac and adapt it to your need).

Of course, if you are interested in moving the flac project further, then you'd be most welcome to contribute to implementing support for LPC encoding of audio samples. The person with most experience on LPC encoding is @wsc1. I'm sure if you asked William specific questions related to LPC encoding, he'd be glad to help you get up to speed.

Cheers,
Robin

Edit: as a workaroud for now, you could probably pin to commit 306014a in your go.mod. At least, with that commit there should be working encoding of metadata (but not yet any attempt at supporting encoding of audio samples).

Hi @mewmew
I also want to just edit metadata leaving the audio untouched and was already using the flac.Encode() helper.
I'm trying to pin 306014a but it doesn't work due to unsynced dependencies I think (Errors like bitio.Writer does not implement io.Writer (Write method has pointer receiver))
Is there any chance of having a working branch with the old flac.Encode() for legacy purposer or having an examble like https://go.dev/play/p/P3efeHuhHO but using NewEncoder?
Thanks in advance!

Hi @oprietop!

I also want to just edit metadata leaving the audio untouched and was already using the flac.Encode() helper.

Hopefully, the (unsupported) metadata encoding API (flac.Encode) should still work, after fixing the build that is.

The enc.go file (which was removed in #32), implemented metadata encoding. We've re-added it to an unsupported branch called unsupported-metadata-encoding.

Feel free to try and pin against this branch to use the metadata encoding API.

The only change we've done is to change the type of the bw field from bitio.Writer to *bitio.Writer in the encoder struct to fix the build (62f10a5#diff-5058624b6c704de7e48133a21cbbff8f5745fa67075f6558c08825adf249babcR92).

Hopefully the metadata encoding should still work! We've not tested the metadata encoding of 62f10a5 locally ourselves.

So feel free to test it out and report back.

Wish you happy coding and a lovely winter.

Cheers,
Henry & Robin

Thank you both for the spoon-feeding there.

mewmew commented

Hi @oprietop and @spbkaizo,

As of version v1.0.10 of mewkiz/flac support for LPC encoding of audio samples has been added (see #35 and #66).

Cheerful regards,
Henry & Robin

Edit: for an example of how to encode a FLAC stream (including audio samples), see

// Open encoder for FLAC stream.