Seems there's a problem with the high-level API..

First off, the mmap() function in madplay was failing for some reason. So I added an open() and used that filedescriptor rather than the stdio's fd. Running it generated this error, which is what I also see when I try to use the high-level API with my code :

minimad > /tmp/r decoding error 0x0101 (lost synchronization) at byte offset 0

I modified minimad again, this time to ignore that error. The output is identical to what I heard when I was doing the decoding with my application.

However, I also tried using madplay (raw output format), and it worked great. Audio sounded perfect. So, what do you think the problem could be with the high level API?

Thanks, Reza

Rob Leslie wrote:

Does minimad run correctly, and can you play the resulting audio without loud clicks? If so, it would seem MAD is working.

You might be having difficulty with MAD's stream API. You might want to have a look at Bertrand Petit's madlld for further guidance. His demo uses the low-level API rather than the high-level one, but you can extrapolate from it what you should be doing in the input callback.

http://www.bsd-dk.dk/%7Eelrond/audio/madlld/

-- Rob Leslie rob@mars.org

Rob Leslie wrote:

In order for minimad to use mmap(), it's necessary to invoke it like this:

minimad <input.mp3 >output.pcm

That's how I did invoke it, before the change. It still failed. Perhaps because the location of the mp3 file is nfs mounted, though that's just a guess.

I don't think the problem is with the high-level API since both minimad and madplay use it.

Perhaps the problem is the byte order of the output samples. minimad always outputs little-endian samples, but madplay's raw output uses the native endian format of your host. Is your audio driver expecting big-endian samples?

To test this, I went ahead and forced madplay to little-endian :

/* # if defined(WORDS_BIGENDIAN) # define audio_pcm_s32 audio_pcm_s32be # define audio_pcm_s24 audio_pcm_s24be # define audio_pcm_s16 audio_pcm_s16be # else */ # define audio_pcm_s32 audio_pcm_s32le # define audio_pcm_s24 audio_pcm_s24le # define audio_pcm_s16 audio_pcm_s16le

//# endif

It played the audio just fine. Perhaps it has something to do with minimad's scale() function (I'm currently using it as-is, in my code)? I might try ripping out some of the audio massaging code from audio_raw.c and putting into my code to see what it does. Otherwise, do you have any other thoughts?

One other question -- with the high-level API's call of mad_stream_buffer(), does it want data in a prefered size? Or does it have to end on some multiple of some size?

Thanks, Reza

I answered this in private email: I think the problem is that you are refilling your stream buffer without copying the unconsumed data from the end of the previous buffer.

Right on the money -- I changed the code to accomodate, and it seems to be working now. I actually pre-fetch blocks of mp3 into buffers via a seperate thread, so copying them around wouldn't work -- Instead, I have the buffers overlap by 2880+MAD_BUFFER_GUARD bytes. Works like a charm.

So, now, it actually gets through the entire MP3 before it dies. However, it still sounds like shit. It makes a clicking/swooshing sound at about 4Hz, but you can clearly hear the audio in the background. Could this be a result of the simplified scale() function in minimad (which I'm currently using?) Here are all the specs I have to go on for the rio's audio device (from some source code) :

// Must output buffers of 4608 bytes or audio driver barfs. // Sample rate is always 44.1k, format always 16-bit stereo, alternating // left/right samples.

and this is the contents of /proc/audio

buffers : 32 buf size : 4608 samples : 27767808 interrupts: 0 wakeups : 0 fifo errs : 0 buffer hwm: 31 first fill: 1 below half: 18

Thanks, Reza

Output of what? The output of scale is a signed int, which then gets copied to a char array, then to a ringbuffer..

int rio_mad_output(void *ptr, struct mad_header const *header, struct mad_pcm *pcm) { unsigned int nchannels, nsamples; mad_fixed_t const *left_ch, *right_ch; char *outbuffer; int i; signed int sample; r_glue *g = ptr;

/* pcm->samplerate contains the sampling frequency */

if (g->decode_status == DECODE) { nchannels = pcm->channels; nsamples = pcm->length; left_ch = pcm->samples[0]; right_ch = pcm->samples[1];

outbuffer = (char*) malloc(sizeof(char) * nsamples * nchannels * 2); [ ..sanity checks.. ] for (i=0; i<(nsamples*4); i+=4) { sample = rio_mad_scale(*left_ch++); outbuffer[i] = (sample >> 0) & 0xff; outbuffer[i+1] = (sample >> 8) & 0xff; if (nchannels == 2) sample = rio_mad_scale(*right_ch++); outbuffer[i+2] = (sample >> 0) & 0xff; outbuffer[i+3] = (sample >> 8) & 0xff; } rio_ringbuffer_write(g->ringbuffer, outbuffer, nsamples*nchannels*2); free(outbuffer); return MAD_FLOW_CONTINUE; } else if (g->decode_status == CLEAR) { return MAD_FLOW_STOP; } return MAD_FLOW_BREAK;

}

It's probably something simple that I'm missing, and that's the worst part of it.

Reza

Joe Drew wrote:

On Mon, 2002-05-27 at 18:01, Reza Naima wrote:

...

However, it still sounds like shit. It makes a clicking/swooshing sound at about 4Hz, but you can clearly hear the audio in the background. Could this be a result of the simplified scale() function in minimad (which I'm currently using?)

Probably not. The scale() function isn't optimal, but isn't *that* bad.

I remember something similar happening with mpg321. Make sure you're using signed ints for output (I believe signed is the correct one). It's possibly just a one-bit error in all sound.

On Mon, 2002-05-27 at 23:40, Reza Naima wrote:

All of which have a length of 4806 bytes, and happen at a regular interval of every 18432 (0x4800) bytes.

Are you preserving the data which has not been decoded, and needs to be preserved for the next call to mad_stream_buffer? When you don't, it causes seams and blips and pops.

The following snippet of code is from mpg321, mad.c:read_from_fd().

int bytes_to_preserve = stream->bufend - stream->next_frame;

/* need to preserve the bytes which comprise an incomplete frame, that mad has passed back to us */ if (bytes_to_preserve) memmove(playbuf->buf, stream->next_frame, bytes_to_preserve);

if( !(read(playbuf->fd, playbuf->buf + bytes_to_preserve, BUF_SIZE - bytes_to_preserve) > 0) ) playbuf->done = 1;

mad_stream_buffer(stream, playbuf->buf, playbuf->length);

Not that anyone really cares, but I finally figured out the problem. And I spent a good deal of time hitting myself in the head for not noticing it sooner. The decoding worked fine, but rather the ringbuffer i put the decoded audio into had a slight bug. When it looped, it skipped over a byte of data (a ++i where I should have had a i++), and thus caused the offset you see below.

Thank you all for all the great help & support.

Reza

Reza Naima wrote:

Woot.. Making progress, finally -- though I'm not happy I'm having to resort to this. I did a byte by byte comparison of the madplay output and the output of my code... It's identical up to byte 0x4800 ...

My code :

00047f0 0095 ff69 0199 fe51 00d1 fedf ff20 006a 0004800 6600 2afe fa01 b5fe 9800 38ff 3400 a1ff 0004810 2500 95fe a901 0ffd 8302 74fe 1801 1f00 0004820 1000 1b01 e0ff 2300 43ff d200 03ff 3100 0004830 fa00 0eff ab00 05ff 4b00 4eff 6500 66ff

madplay :

00047f0 0095 ff69 0199 fe51 00d1 fedf ff20 006a 0004800 fe66 012a fefa 00b5 ff98 0038 ff34 00a1 0004810 fe25 0195 fda9 020f fe83 0174 0018 001f 0004820 0110 ff1b 00e0 ff23 0043 ffd2 0003 0031 0004830 fffa 000e ffab 0005 ff4b 004e ff65 0066

It remains out of sync until 0x0005a00...

The ranges it falls out of sync are :

04800 - 05A00 09000 - 0A200 0D800 - 0EA00 ....

All of which have a length of 4806 bytes, and happen at a regular interval of every 18432 (0x4800) bytes. Every time I get a callback to the output function, I get sent 4806 bytes -- this is also the same size as the buffer on the audio device, so I'm gessing the number isn't random.. Now to figure out what? As the pattern is regular, I could potentially compensate for it, but that seems plain ugly..

-r

On Tuesday, May 28, 2002, at 10:37 AM, Robin O'Leary wrote:

On Mon, May 27, 2002 at 12:04:21AM -0700, Rob Leslie wrote:

...

... I think the problem is that you are refilling your stream buffer without copying the unconsumed data from the end of the previous buffer. ...

Is there a good reason for this requirement?

One of the goals of the stream API was to avoid "owning" the data passed to it. The stream just maintains a set of pointers within a buffer or memory mapping which you provide. This minimizes data copying (or rather, places it under the control of your application) and avoids other overhead, and is consistent with the minimalist nature of the library.

How many places go fishing around in the stream struct directly? It seems mad_bit_read() and friends are almost exclusively responsible for furnishing input data to the rest of the library; mad_header_decode() does some direct manipulation too. If these functions were taught how to get bytes from the previous stream buffer until it was fully `used up', before starting on the current one, could we then have a new function, say mad_stream_buffer_append(), which would leave the bitptr struct intact counting down bits from the last buffer, and hence avoid the need for the caller to do memmove()? Would the overhead of checking the number of bits left in those low-level access routines be prohibitive?

I'm reluctant to add overhead to the bit reading routine that would make it any slower than it is already. It's for this reason that libmad requires the stream to contain complete, contiguous frames.

I think the best approach, if you really want the kind of API you describe, is to write an abstraction layer on top of libmad that implements it. Such a library could also include other useful utilities such as PCM formatting, resampling, dithering, etc. Arguably the high-level API provided by libmad should have been a part of this hypothetical library also since it is effectively layered on top of the low-level API.

-- Rob Leslie rob@mars.org