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

Michael Niedermayer michaelni
Thu Oct 14 21:13:03 CEST 2010


On Wed, Oct 13, 2010 at 07:52:12PM -0400, Justin Ruggles wrote:
> 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.

old API decoders surely dont need to copy with old API.
old API decoders surely dont need to copy with new API if the api can provide
a buffer to the decoder (this can be through function argument like its done
currently)
new API decoders surely dont need to copy with new API because otherwise the
API sucks and needs more work
whats left is new API decoders and used with old API and for this get_buffer()
should return the user supplied buffer if its large enough and fail if its not
large enough.
The case where the user overrides get_buffer() and supplies a user specified
buffer which its own code doesnt use is a case that id consider user error.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101014/4a11a3e2/attachment.pgp>



More information about the ffmpeg-devel mailing list