I found a reasonable-looking open source mpg123 AudioFormat here:
https://code.google.com/p/hifiaudioplayer/source/browse/trunk/TestApplication/MP3AudioFormat.h and https://code.google.com/p/hifiaudioplayer/source/browse/trunk/TestApplication/MP3AudioFormat.cpp
It is however GPL so I can’t directly use it for this commercial project.
Unless someone has a free audio format with LGPL or similar licensing, I’m going to write my own and then make it open/free. Stay tuned!
I definitely need to write something new anyway, as this one isn’t thread-safe (it has a single static instance of a file it’s reading so it’ll definitely die if you happen to read two MP3 files at once). Stay tuned!
I will be pending.
yo estare pendiente
Made improved MP3AudioFormat class from hifiaudioplayer.
No more single static instance of a file (actually is not a file, it’s only filename), direct reading from Juce InputStream.
Thanks for the post Eugene!
I assume you have to move the mpg123_init and mpg123_exit to a static place to keep the library available for simultaneous mp3 files to be read, Is that true?
[quote=“justinb”]Thanks for the post Eugene!
I assume you have to move the mpg123_init and mpg123_exit to a static place to keep the library available for simultaneous mp3 files to be read, Is that true?[/quote]
Oh, sorry. :oops:
Forgot about mpg123_init and mpg123_exit, because currently i use single thread. 
Made simple singleton class with double checked locking for mpg123_init and mpg123_exit.
Now possible to initialize library automatically at first usage or manually at program startup using MP3AudioFormat::InitMP3Library().
P.S. But i’ve don’t tested this on multiple threads.
Good stuff! I went into it expecting that you hadn’t dealt with seeking issues, but using mpg123_replace_reader is definitely the way to go!
I’m actually trying to sort of “clean room” create my own reader - because unfortunately any code directly derivative of the original is still GPL’ed and thus can’t be used in my commercial, closed-source project. So stay tuned anyway… 
BTW, I’m pretty sure you’ve fixed the potential threading issue I saw - because there seem to be no static variables aside from constants. But the fact that you can seek in the file without having to keep it all in memory is even better.
Now that someone else has looked at this code, I have a question - or an idea, or something.
Look at the original, lines 255-256 - you have the same code.
It seems to me that this is far too complicated for what it does. Here’s the code with a little extra whitespace… [code]sampleBuffer[0][j] =
readBuffer[j * byteAlign + 3] * 0x1000000 +
readBuffer[j * byteAlign + 2] * 0x10000 +
readBuffer[j * byteAlign + 1] * 0x100 +
readBuffer[j * byteAlign + 0];
sampleBuffer[1][j] =
readBuffer[j * byteAlign + 7] * 0x1000000 +
readBuffer[j * byteAlign + 6] * 0x10000 +
readBuffer[j * byteAlign + 5] * 0x100 +
readBuffer[j * byteAlign + 4];[/code]
First, I claim that this code only makes sense if “byteAlign” is always 8. If it’s less than 8, samples will overlap in the buffer - if greater than 8, there will be gaps.
Second, I claim that byteAlign should really be called something like sizeOfTwoSamplesInBytes - so all the samples here are assumed to be 4 bytes long, and all the ints we’re reading should really be int32s (I know on most processors we are running on, int and int32 are the same, but this isn’t universally true.
Now, let’s look at the definition of readBuffer:unsigned char readBuffer[65536 * 4 * 2 * 2 * sizeof(int)]; and suppose we changed this to unsigned int32 readBuffer[65536 * 4 * 2 * 2]; The “* sizeof(Type)” in a malloc or array is a code smell that often results in a refactoring like this…
Now the long lines of code above become… sampleBuffer[0][j] = readBuffer[2 * j];
sampleBuffer[1][j] = readBuffer[2 * j + 1]; But then I can apply that same refactoring again to readBuffer, which is really a two-dimensional array… unsigned int32 readBuffer[65536 * 4 * 2][2];
// ...
for (int j = 0; j < samps; j++)
for (int i = 0; i < 2; ++i)
sampleBuffer[i][j] = readBuffer[j][i]; - just a matrix transposition!
I haven’t actually tried this on your code yet. You implied you were testing it, what are you doing to test, are you running it within this guy’s application?
In fact, the original code isn’t strictly right because of endianness - the code assumes that the samples are “little-endian” which is true of most but not all machines. If you run this code on a big-endian machine, you’ll get garbled samples… though I don’t think any of the platforms that Juce now supports are big-endian.
But why do all this arithmetic, anyway? Just do the matrix transposition.
The final question is whether the matrix transposition itself is necessary. Unfortunately, Juce’s sample buffer has a different memory layout than mpg123s - in JUCE, each track is a separate, continuous track of data, whereas in mpg123 the tracks are interleaved.
I’ve just glimpsed at the code, may be the idea is to solve an endianess problem. May be the files (streams) are of a definite endianess and the code here ensures that whatever the endianess the machine is, it will read it correctly ?
unsigned int32 readBuffer[65536 * 4 * 2][2];
// ...
for (int j = 0; j < samps; j++)
for (int i = 0; i < 2; ++i)
sampleBuffer[i][j] = readBuffer[j][i];
This code don’t work for mono files.
This code sounds better.
int align;
int readBuffer[65536*4*2*2]; // or int32
...
align=numChannels*4; // in class constructor
...
while(numToRead>0){
size_t samples=0; // "int" is not safe for x64
int errorCode=mpg123_read(mh,(unsigned char*)readBuffer,numToRead*align,&samples);
samples/=align;
int* rd=readBuffer;
Array<int*> cdata;
for(int i=0;i<(int)numChannels;++i)
cdata.add((int*)reservoir.getSampleData(i,offset)); // temporary variable "sampleBuffer" is not needed, direct copy samples to "reservoir"
for(int j=0;j<(int)samples;++j)
for(int i=0;i<(int)numChannels;++i,++rd)
cdata[i][j]=*rd;
numToRead-=(int)samples;
offset+=(int)samples;
// checking errors at the end, because buffer can contain decoded samples (with error codes and MPG123_DONE code)
if(errorCode!=MPG123_OK || samples==0)
break;
}
...Also mpg123_replace_reader_handle is safer than mpg123_replace_reader.
mpg123_replace_reader_handle(mh,read_juce_stream,seek_juce_stream,0);
mpg123_open_handle(mh,input);
...
static ssize_t read_juce_stream(void *h, void *pBuf, size_t nSize);
static off_t seek_juce_stream(void *h, off_t offset, int whence);Tested with MSVC Win32 & Win64. Works fine.
I did contemplate this - but I don’t believe that’s the case.
The files themselves might well have a specific endianness - but the AudioFormat never sees the data from the files - instead it sees the stream of samples being output from mpg123. My belief is that this stream of output samples is in the correct endianness for the machine that mpg123 is running in - the code seems to imply this, but more, mpg123 would simply be broken on such machines if it weren’t the case! (Because if it were true, then on a big-endian machine, before you output the mpg123 data, you’d need to run it through some program of your own to fix the endianness of the samples.)
Unfortunately, I have no big-endian machine to test on, so this is all hypothesis. At the end, I’ll ask the mpg123 developers if my surmise is correct but I’d be surprised if it were wrong.
I was refactoring the code - i.e., producing code that generated exactly the same output as the previous code - so any previous bugs will still be there.
I don’t want to spend too much time on the GPL’ed version, as I want to create my own, free-er version I can use in commercial code.
Hey Tom ! Any progress on this ?
Well, I finished it a few months ago, and posted the results here!
http://ax.to/mpg123 is the main project. It isn’t properly componentized, doesn’t have a configure or install, it’s just code - however, there are fairly complete unit tests and I tried to keep the quality high.
If you wanted to use this for yourself, I’d suggest getting the main program compiling and running on your system and go from there!
I must have missed it ! Looks good so far ! Is it under LGPL licence ?
Even better, it’s under the IDC license:
Note that it does NOT write mp3s - because near as I can see, mpg123 doesn’t let you write mp3s either.
I’m using this in production code that will be going out soon so I welcome any issues you find with it.
In particular, if you have pathological or strange mp3s that cause issues, please please send them to me! I want to have a collection of “hard-to-deal-with” mp3s to run my stuff on.
Haha, that’s the only fun licence I ever read . Ok will do !
But it still relies on libmpg123, so doesn’t this mean it is GPL after all (please correct me if I’m wrong…) ?
I’m using libmpg123 and you can find me in #mpg123 on Freenode. It is in fact LGPL, so you can link to it as a dynamic library (.DLL) for a proprietary application.
At some point I will need to implement my own mp3 decoder and when that happens I will likely release it under an MIT license but for now I suggest dynamic linking with mpg123.
Exccellent, thanks!
