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: implement support for flac decoding #9

Merged
merged 3 commits into from
Nov 11, 2017
Merged

Conversation

mewmew
Copy link
Contributor

@mewmew mewmew commented Nov 9, 2017

NOT YET READY FOR MERGE.

This is an initial PR which adds decoding support for FLAC to beep.

My brother and I have been able to play all the FLAC music we've managed to come across, so decoding and playback seems to work well :) (Example playback program at play.golang.org: https://play.golang.org/p/2RafKiP7Cy)

What is left to do is to implement support for seeking. As of yet, we do not know how to do that in a clean way; given that FLAC is a streaming format with potentially dynamic size of audio frames.

Preliminary work to add seek to the FLAC backend is underway in the dev branch. However, the current solution is very naiive and does not take sync codes into accounts. Any help in this direction would be much appreciated.

Cheers,
/@karlek and @mewmew

@mewmew
Copy link
Contributor Author

mewmew commented Nov 9, 2017

Please note, it is a design decision to keep the back-end separate from the beep repository (which is considered a front-end). This way, the same back-end FLAC library can be used by several front-ends. Currently there exist support in Azul3D, and future support for go-audio/audio is planned.

However, all the code is in the public domain, so if it makes sense to include the back-end code in the beep repository, feel free to do so. Perhaps under vendor or with dep or something.

@faiface
Copy link
Owner

faiface commented Nov 10, 2017

Awesome!

Regarding seeking, after taking a quick look (so this is probably nothing new to you), FLAC format claims to enable fast seeking, so it must not be so hard. Here https://xiph.org/flac/format.html it's roughly described how seeking can be done. That's all help I can provide right now. In a few weeks, I will have more time and be able to try and implement it, if necessary.

If everything except for seeking works fine, I am open to merging this PR even now and keep the seeking unimplemented for the time being. It's still possible to load the whole file into a buffer and seek there.

So, what do you think? Will you be able to implement the seeking in a few days/weeks, or should we merge now and do seeking later?

@mewmew
Copy link
Contributor Author

mewmew commented Nov 11, 2017

Awesome!

Regarding seeking, after taking a quick look (so this is probably nothing new to you), FLAC format claims to enable fast seeking, so it must not be so hard. Here https://xiph.org/flac/format.html it's roughly described how seeking can be done. That's all help I can provide right now. In a few weeks, I will have more time and be able to try and implement it, if necessary.

If you'd like to take a stab at implementing seek, you'd be most welcome to :) The issue to implement seek support in the FLAC back-end is located at mewkiz/flac#16

If everything except for seeking works fine, I am open to merging this PR even now and keep the seeking unimplemented for the time being. It's still possible to load the whole file into a buffer and seek there.

So, what do you think? Will you be able to implement the seeking in a few days/weeks, or should we merge now and do seeking later?

You can definitely merge this as is. If anything, it may help iron out bugs as more users with different FLAC files can stress test various parts of the library.

Perhaps mark it as work in progress in the doc comment of the package, and mention that seek support is not yet implemented.

Btw, I've been playing around with pixel lately. It is really a quite lovely package you've build. I offer my sincere gratitude.

Cheers /u

@faiface
Copy link
Owner

faiface commented Nov 11, 2017

Ok, I'll merge now and add a notice about the unimplemented features. Hope to get them implemented soon! (Either by you, or by me).

And thanks for the nice comments on Pixel :). Although I'm still not really satisfied with Pixel from the design standpoint (I'm much more satisfied with Beep). And I sort of know how I'd like to change some things, but ugh, time and motivation. And many things that are planned to get into Pixel but still aren't there. I'm still planning on "recruiting" a bunch of contributors for Pixel and I will execute this plan someday.

@faiface faiface merged commit c723881 into faiface:master Nov 11, 2017
@mewmew
Copy link
Contributor Author

mewmew commented Nov 13, 2017

And thanks for the nice comments on Pixel :). Although I'm still not really satisfied with Pixel from the design standpoint (I'm much more satisfied with Beep).

I was very pleasantly surprised when I stumbled across the beep API, and it took some time to grok the power of it. But that was certainly a rewarding experience, especially for someone who is interested in but havn't played to much with functionally inspired APIs.

Really glad you decided to write up your design ideas into a more high-level picture; https://faiface.github.io/post/how-i-built-audio-lib-composite-pattern/

With you happy hacking! :)

Cheers /u

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