[vorbis-dev] Problem with ov_read_float()

[David Mitchell]

On Wednesday 16 January 2002 09:14 pm, volsung at asu.edu wrote:
> On Wed, 16 Jan 2002, David Mitchell wrote:
> > I'm having some sort of problem using ov_read_float(). Everything looks
> > good to me, but I'm trashing memory somehow, so clearly I'm screwing
> > something up. What confuses me is why it takes a ***float for the
> > buffer.
>
> There are some problems with the semantics of ov_read_float().  It works as
> defined, but we plan to change a few things before RC4 to make it more
> useful. (ReplayGain on ogg123 will not be committed until the changes
> occur.)
>
> > float **buffer
> > bytes_read = ov_read_float(&vf, &buffer, 0)
> > memcpy (b, *buffer, 0);
>
> Uh, why are you memcopy()ing 0 bytes?  That looks wrong already.

Oops. Thats just a typo in my email. Sorry :-)

>
>
> Some of your confusion might come from a misunderstanding of what points at
> after you call ov_read_float().  buffer is actually an array of floating
> point arrays, one for each channel of audio.  If you are decoding stereo,
> buffer[0] will be the array of left channel samples, and buffer[1] will be
> the right channel.  As you might expect, buffer[0][0] will be the first
> sample in the left channel, and buffer[1][0] will be the first sample in
> the right channel.

OK. That makes sense. I had managed to keep my app from crashing by
basically chopping the inner loop out of ov_read() where the array syntax
is more evident.

>
> Because these are arrays of arrays, you can't just memcpy() the whole thing
> anyway since it doesn't necessarily exist in contiguous memory. 

memcpy() is much faster of course, but since I'll be touching all of the 
samples anyway eventually for replaygain it's not a big deal.

> Also, note
> that bytes_read is a misnomer.  This function actually returns the number
> of samples read (in either channel, not both channels added together).

Ahh, I was suspecting this. Yeah, that really screws things up if you assume 
it's bytes. 

As an aside, I've bitten myself a bunch by mis-using the term "samples", and 
messing up my buffer sizes. I've been using the term "frames" to represent
the number of samples divided by channels. It helps keep me from using
the wrong value. I don't know if you want to overload the word "frame"
since it also has meaning in terms of larger chunks of the Ogg stream. But
when I think about how many "samples" are in a buffer, I think of
bytes_in_buffer / sizeof(int16|float) depending on what's appropriate.

>
> Also make sure that you don't free() buffer anywhere since you don't own
> it.

I've made about every other mistake so far in switching from ov_read() to
ov_read_float(), but haven't managed that one yet ;-)>

>
> [Disclaimer: If this seems odd at all to you, realize that we agree.  The
> fixed version will have semantics closer to ov_read().]

I think the way it works is OK, just touch up the documentation. There isn't 
any way to know that buffer is a pointer to an array of pointers to arrays
versus a pointer to a pointer to an array just from looking at 
"float ***buffer". And since audio is often passed around as a one
dimensional array with the channels interleaved, that's what I assumed
was being used here. That could be easily cleared up by just expanding the
"Typical usage" section. Maybe something like:

float **buffer;
float *left, *right;
float *output;

int32 frames = ov_read_float(&vf, &buffer, 0);
left = buffer[0];
right = buffer[1];
int32 j = 0;

for (int32 i = 0; i < frames; i++) {
        output[j++] = left[i];
        output[j++] = right[i];
};

It's a little verbose, but fairly clear.

-David Mitchell

>
> ---
> Stan Seibert
>
>
> --- >8 ----
> List archives:  http://www.xiph.org/archives/
> Ogg project homepage: http://www.xiph.org/ogg/
> To unsubscribe from this list, send a message to
> 'vorbis-dev-request at xiph.org' containing only the word 'unsubscribe' in the
> body.  No subject is needed. Unsubscribe messages sent to the list will be
> ignored/filtered.

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to 'vorbis-dev-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.