[FFmpeg-devel] [PATCH] atrac1 decoder and aea demuxer

[Benjamin Larsson]

Vitor Sessak wrote:
> Benjamin Larsson wrote:
>> Revision 5 or something, changes to previous patches are:
>>
>> aea demuxer:
>> Better probe logic
>>
>> atrac1 decoder:
>> Uses mdct_half now
>>
>> Still missing is: the documentation patch, will be added when the code
>> is ok and the split out of the atrac common transform code, will be
>> fixed when the decoder code is ok.
>
> Just a couple of comments...
>
>> +typedef struct {
>> +    GetBitContext       gb;
>
> This is used only in one function, can be a local var.

Ok.

>
>> +        /* copy the 2nd half of the transformed samples into the 
>> prev_frame buffer */
>> +        /* so it can be used as reference by the following frame */
>> +        memcpy(&su->prev_spec[ref_pos], 
>> &q->long_buf[band_samples/2], sizeof(float) * band_samples/2);
>
> Can this memcpy() be avoided by swapping buffers?

The the cost of some more memory it should be avoidable.

>
>> +    /* round, convert to 16bit and interleave */
>> +    if (q->channels == 1) {
>> +        /* mono */
>> +        for (i = 0; i<AT1_SU_SAMPLES; i++)
>> +            samples[i]     = av_clipf(q->out_samples[0][i], 
>> -32700./(1<<15), 32700./(1<<15));
>
> DSPContext.vector_clipf()

Ok.

>
>> +static av_cold void init_mdct_windows()
>> +{
>> +    int i;
>> +    /* Generate the short window */
>> +    ff_sine_window_init(short_window,32);
>> +
>> +    /** The mid and long windows uses the same sine window splitted
>> +     *  in the middle and wrapped into zero/one regions as follows:
>> +     *
>> +     *                   region of "ones"
>> +     *               -------------
>> +     *              /
>> +     *             / 1st half
>> +     *            / of the sine
>> +     *           /  window
>> +     * ---------/
>> +     * zero region
>> +     */
>
> I think it has already mentioned in a previous review and cannot be 
> done cleanly, but it would be nice not to spend cycles multiplying by 
> one or zero...

I did a version with that but it was slower so I won't revisit that again.

>
> -Vitor

MvH
Benjamin Larsson