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

Michael Niedermayer michaelni
Sat Oct 16 02:29:02 CEST 2010


On Fri, Oct 15, 2010 at 06:35:01PM -0400, Justin Ruggles wrote:
> Justin Ruggles wrote:
> 
> > Michael Niedermayer wrote:
> > 
> >> 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.
> > 
> > I think I might have been misinterpreting the API.  For video decoders,
> > what does it mean as far as buffer allocation when CODEC_CAP_DR1 is not set?
> 
> So I think I have this worked out and I don't see how we can avoid a
> memcpy with the old API when CODEC_CAP_DR1 is not set.  There would be
> no other way to get the data into the correct output buffer.
> 
> other questions:
> 
> 1. Should AVCodecContext.user_buffer be supported for video decoders?

possible but this is seperate, lets not entangle this patch with too many
other changes


> If so, should it be user_buffer[4] and user_buffer_size[4]?

possible


> 

> 2. avcodec_default_get_buffer() supports allocating multiple internal
> buffers.  How should that be handled if the buffer is supplied by the
> user?  Don't support multiple buffers?  Use the user-supplied buffer
> just for the first one?

there are buffer hints (grep for hint in avcodec.h) that specify if a buffer
will be reused/read/preserved/blah
the user supplied buffer is likely just valid for this call and cannot be used
for some cases of the hints. For what remains using the buffer on the first
call only seems ok

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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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/20101016/19049d25/attachment.pgp>



More information about the ffmpeg-devel mailing list