[FFmpeg-devel] [PATCH+RFC] AVFrame for audio

[Justin Ruggles]

Michael Niedermayer wrote:

> On Wed, Oct 06, 2010 at 11:05:26AM -0400, Justin Ruggles wrote:
>> Michael Niedermayer wrote:
>>
>>> On Tue, Oct 05, 2010 at 04:55:12PM -0400, Justin Ruggles wrote:
>>>> Michael Niedermayer wrote:
>>>>
>>>>> On Wed, Sep 29, 2010 at 09:20:04PM -0400, Justin Ruggles wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Peter Ross wrote:
>>>>>>
>>>>>>> On Thu, Sep 02, 2010 at 07:11:37PM -0400, Justin Ruggles wrote:
>>>>>>>> Hi,
> [...]
>>>>> [...]
>>>>>> @@ -644,29 +677,49 @@
>>>>>>  }
>>>>>>  #endif
>>>>>>  
>>>>>> +#if LIBAVCODEC_VERSION_MAJOR < 53
>>>>>>  int attribute_align_arg avcodec_decode_audio3(AVCodecContext *avctx, int16_t *samples,
>>>>>>                           int *frame_size_ptr,
>>>>>>                           AVPacket *avpkt)
>>>>>>  {
>>>>>> +    AVFrame frame;
>>>>>> +    int ret, got_frame = 0;
>>>>>> +
>>>>>> +    avcodec_get_frame_defaults(&frame);
>>>>>> +
>>>>>> +    ret = avcodec_decode_audio4(avctx, &frame, &got_frame, avpkt);
>>>>>> +
>>>>>> +    if (ret >= 0 && got_frame) {
>>>>>> +        *frame_size_ptr = frame.nb_samples * avctx->channels *
>>>>>> +                          (av_get_bits_per_sample_format(avctx->sample_fmt) / 8);
>>>>>> +
>>>>>> +        /* ensure data will fit in the output buffer */
>>>>>> +        if (*frame_size_ptr > AVCODEC_MAX_AUDIO_FRAME_SIZE) {
>>>>>> +            av_log(avctx, AV_LOG_WARNING, "avcodec_decode_audio3 samples "
>>>>>> +                   "truncated to AVCODEC_MAX_AUDIO_FRAME_SIZE\n");
>>>>>> +            *frame_size_ptr = AVCODEC_MAX_AUDIO_FRAME_SIZE;
>>>>>> +        }
>>>>>> +
>>>>>> +        memcpy(samples, frame.data[0], *frame_size_ptr);
>>>>>  the default get_buffer() should return the appropriate
>>>>> buffer for this case.
>>>> I'm sorry, I don't understand your comment.
>>> i mean (non functional psseudocode below to explain the idea)
>>> avcodec_decode_audio3(){
>>>     avctx->foobar= samples;
>>>     ret = avcodec_decode_audio4(avctx, &frame, &got_frame, avpkt);
>>>     ...
>>>     assert(samples == frame.data[0]);
>>>
>>> -----
>>> default_get_buffer(){
>>>     if(avctx->foobar)
>>>         return avctx->foobar;
>>>
>>>
>>> (and yes this cannot work for all theoretical decoders)
>>
>> I think I get it.  So avctx->foobar would be an optional user-supplied
>> buffer (avctx->user_buffer?) that default_get_buffer() would return if
>> it is non-NULL, right?
> 
> yes
> 
> 
>> The problem I see is this:
>> avcodec_decode_audio3() would use avcodec_decode_audio4().
>> avcodec_decode_audio4() allocates as large a buffer as is needed through
>> get_buffer(), but the avcodec_decode_audio3() API only requires the
>> user-supplied buffer to be AVCODEC_MAX_AUDIO_FRAME_SIZE.  Couldn't this
>> lead to the decoder writing past the end of a user-supplied buffer if it
>> isn't large enough?  I guess we could also add a field
>> avctx->user_buffer_size?
> 
> yes, of course
> it was just a rough idea ...

I'm running into some questions trying to implement the rough idea.  The
only way I can see this working smoothly is if avcodec_decode_audio3()
always sets get/release_buffer to default.  Also, either all audio
decoders will have to support CODEC_CAP_DR1 (i.e. they always use
get/release_buffer) or there needs to be a fallback that will memcpy
into the user buffer if CODEC_CAP_DR1 is not supported.

-Justin