How come no in-memory vorbis?

[raub]

Hello!

I use LabSound as a backend for Node.js bindings. LabSound uses libnyquist to decode audio. For JS I export WebAudioApi-compliant set of classes.

Among other things it is very important to do in-memory decoding. Because all JS examples and docs provide separate means of loading and decoding for audio files. Namely XHR for webpages, and I use fs.readFile on Node.js. I'm currently trying to reproduce a simple case with file loading. And I found out that LabSound and specifically libnyquist is incapable of doing ogg in memory.

void VorbisDecoder::LoadFromBuffer(AudioData * data, const std::vector<uint8_t> & memory)
{
    throw LoadBufferNotImplEx();
}

But there is, so to speak, a wae. I'll try to fit it into the VorbisDecoder. If I succeed, I'll do a PR.

Any thoughts on this? libnyquist doesn't seem to be developed too actively these days, are you still going to continue maintaining it?

[ddiakopoulos]

Hi @raub -- check out this fork with some additional changes for in-memory loading: https://github.com/modulesio/libnyquist/commits/master

@modulesio hasn't sent me any pull requests for these changes, but I'd like to re-integrate them. I also see you have a PR open for this which I'll take a look at later.

In terms of maintaining libnyquist, it's on my list of activities later this year to give the library some love :)

[raub]

Hey @ddiakopoulos , I've reviewed the VorbisDecoder at modulesio/libnyquist.

I don't know where he got that code from, but it's different from the mentioned stackoverflow solution in some aspects, and yet the same algorithmically. There are few stylistic points I'd like you to be concerned with (and those differ between our implementations):

• Custom callbacks must be private - why would you address them from outside?

• Don't Repeat Yourself principle - the code in both constructors tends to be almost the same.

• Avoid the creation of class properties, that are going to be used only in some usecases - may be storing the source data pointer inside VorbisDecoderInternal is useful for future implementation of "streaming", but now it seems to be a bit of an overkill.

Other than that, if both solutions work, I don't care which one is to be merged. I just need this stuff to work. I'm currently using the binary built with my changes, but I'd prefere to build from master of the root fork.

[ddiakopoulos]

Thanks @raub. Just because I pointed you to that repo doesn't mean I code-reviewed it :)

I've merged your recent PR and made some additional project updates.