[Ffmpeg-devel] [PATCH] qcelp codec supporrt

Måns Rullgård mru
Thu Dec 14 00:18:02 CET 2006


Moriyoshi Koizumi <koizumi at gree.co.jp> writes:

> Hi,
>
> On Tue, 2006-12-12 at 21:36 +0100, Michael Niedermayer wrote:
>> Hi
>> 
>> On Mon, Dec 11, 2006 at 12:21:58PM +0900, Moriyoshi Koizumi wrote:
>> > Hello,
>> > 
>> > I implemented avcodec stub code that glues ffmpeg with the reference
>> > implementation of QCELP which is provided in the 3GPP2 project page.
>> > 
>> > Any comments are welcome.
>> 
>> [...]
>> 
>> > +static const int frame_size_map[16] = {
>> > +     0,  3,  7, 16, 34,  7, -1, -1,
>> > +    -1, -1, -1, -1, -1, -1,  0, -1,
>> > +};
>> 
>> this should fit in int8_t which would save a few bytes
>
> Aren't they aligned to word boundaries by the compiler?

No.

>> > +            *q++ = FFMIN(32768, FFMAX(-32768, samples[i] * 4));
>> 
>> 16bit signed is -32768 .. 32767 not 32768
>> also this could use clip(lrintf()) instead of MIN+MAX
>
> Is it okay to use the function that is part of C99? 

Anything in C99 is considered OK as an initial assumption.  If it's
found to be missing from some system that we deem important, we'll
come up with something then.

>> the packet spliting should be done in an AVParser so that that
>> decoder always gets exactly one packet each time
>
> I put it in the for loop because I didn't realize how the AVParser
> handles it and hands it to the decoder. I'll look into it.

If a demuxer sets the AVStream.need_parsing field an AVParser will be
invoked automatically.

>> > +    avctx->frame_size = FSIZE * 10; /* hmm.. */
>> 
>> each AVPacket should correspond to a single packet unless the packets
>> are all the same size and very small (like for PCM)
>
> Right, but the size of the resulting file was considerably decreased
> when I put as large packets as feasible. Would there be any room for
> optimisation without breaking the rule?

Can frames contain a variable number of samples?

>> > +            *p++ = (pkt.data[i] >> 8) & 0xff;
>> > +            *p++ = pkt.data[i] & 0xff;
>> 
>> the & 0xff isnt needed
>
> I appended it because I thought signed to unsigned conversion
> between two values of the different storage size is not
> defined in the C spefication.

Conversion to unsigned is well-defined.  Conversion to signed is only
defined when the value is within the range of the target type.  This
is mostly because the C standard doesn't specify how negative numbers
are represented.  Apparently there were some machines in the distant
past that used one's complement to represent negative numbers.

-- 
M?ns Rullg?rd
mru at inprovide.com




More information about the ffmpeg-devel mailing list