[FFmpeg-devel] [PATCH] avcodec_decode_audio4

[Justin Ruggles]

On 02/28/2011 03:49 AM, Martin Storsj? wrote:

> On Sun, 27 Feb 2011, Justin Ruggles wrote:
> 
>> I'm sorry this is such a huge patch, but it all needs to be applied at
>> the same time in order to work.
> 
> Since the code seems to be quite much the same that is added to each 
> decoder, would it be possible to keep this at an outer layer until each 
> codec needs to do something different with it?

It's not possible because each codec has a different way of calculating
the number of samples in the frame, and each at a different point in the
decoding process.  Some are fixed values, but others require reading
some frame header data or even more complex things in order to know the
sample count, which is required to allocate the proper amount of data.

> Also, won't this in principle break the ABI of AVCodec (if someone 
> implements a codec outside of lavc), since the decode function suddenly is 
> supposed to behave in a different way, even if the actual signature isn't 
> changed? Although I guess that isn't a supported scenario?

It's the same as when we changed it to decode from AVPackets a couple
years ago.  I don't see any way around this.  If we want to change how
the decoding function works we'll have to break that scenario at some
point.  In this case there is not a backwards-compatible way to handle
it because currently *data_size is expected to be set to the size of the
output buffer prior to calling decode().  When using get_buffer() this
is not possible.

> Also:
> 
>> diff --git a/libavcodec/g722.c b/libavcodec/g722.c
>> index 0efc390..c293f84 100644
>> --- a/libavcodec/g722.c
>> +++ b/libavcodec/g722.c
>>  static av_cold int g722_close(AVCodecContext *avctx)
>>  {
>>      G722Context *c = avctx->priv_data;
>> +    if (avctx->trellis) {
>>      int i;
>>      for (i = 0; i < 2; i++) {
>>          av_freep(&c->paths[i]);
>>          av_freep(&c->node_buf[i]);
>>          av_freep(&c->nodep_buf[i]);
>>      }
>> +    }
>> +
>> +    if (c->frame.data[0])
>> +        avctx->release_buffer(avctx, &c->frame);
>> +
>>      return 0;
>>  }
> 
> I think the if (trellis) around freeing those data structures is 
> unnecessary. The same g722_close function is already used when closing the 
> encoder even if trellis wasn't used.


It seems I messed up the changes for that file.  The decoder and encoder
share the close() function, so I need to split it into 2 parts based on
avctx->codec->encode and avctx->codec->decode.  Changed locally.

Thanks,
Justin