[FFmpeg-devel] [PATCH+RFC] AVFrame for audio
Michael Niedermayer
michaelni
Sat Nov 13 02:10:27 CET 2010
On Mon, Nov 01, 2010 at 05:19:00PM -0400, Justin Ruggles wrote:
> Michael Niedermayer wrote:
>
> > On Sat, Oct 30, 2010 at 08:06:42AM -0400, Justin Ruggles wrote:
> >> Michael Niedermayer wrote:
> >>
> >>> On Thu, Oct 28, 2010 at 06:58:11PM -0400, Justin Ruggles wrote:
> >>>> Michael Niedermayer wrote:
> >>>>
> >>>>> On Wed, Oct 27, 2010 at 10:13:10PM -0400, Justin Ruggles wrote:
> >>>>>> Michael Niedermayer wrote:
> >>>>>>
> >>>>>>> On Tue, Oct 26, 2010 at 09:31:13PM -0400, Justin Ruggles wrote:
> >>>>>>>> Michael Niedermayer wrote:
> >>>>>>>>
> >>>>>>>>> On Sun, Oct 17, 2010 at 05:22:54PM -0400, Justin Ruggles wrote:
> >>>>>>>>>> Michael Niedermayer wrote:
> >>>>>>>>>>
> >>>>>>>>>>> On Sat, Oct 16, 2010 at 04:12:26PM -0400, Justin Ruggles wrote:
> >>>>>>>>>>>> Michael Niedermayer wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>> 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
> >>>>>>>>>>>> I think I've implemented it in a way that will work even when the
> >>>>>>>>>>>> various buffer hints are set. This implementation will not use memcpy
> >>>>>>>>>>>> in avcodec_decode_audio3() in the most common case of the decoder
> >>>>>>>>>>>> supporting CODEC_CAP_DR1, only needing 1 buffer, and not needing a
> >>>>>>>>>>>> buffer larger than AVCODEC_MAX_AUDIO_FRAME_SIZE.
> >>>>>>>>>>>>
> >>>>>>>>>>>> One thing I'm unsure of is whether to truncate output if it is too large
> >>>>>>>>>>>> for avcodec_decode_audio3() (which is done in this patch) or to return
> >>>>>>>>>>>> an error instead.
> >>>>>>>>>>> I think its better to tell the user straight through an error that there is a
> >>>>>>>>>>> problem instead of generating output that contains randomly truncated packets
> >>>>>>>>>> Ok. New patch.
> >>>>>>>>>>
> >>>>>>>>>> -Justin
> >>>>>>>>> [...]
> >>>>>>>>>> int avcodec_default_get_buffer(AVCodecContext *s, AVFrame *pic){
> >>>>>>>>>> int i;
> >>>>>>>>>> int w= s->width;
> >>>>>>>>>> int h= s->height;
> >>>>>>>>>> + int is_video = (s->codec_type == AVMEDIA_TYPE_VIDEO);
> >>>>>>>>>> InternalBuffer *buf;
> >>>>>>>>>> int *picture_number;
> >>>>>>>>>>
> >>>>>>>>>> @@ -235,7 +258,8 @@ int avcodec_default_get_buffer(AVCodecContext *s, AVFrame *pic){
> >>>>>>>>>> return -1;
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> - if(av_image_check_size(w, h, 0, s))
> >>>>>>>>>> + if(( is_video && av_image_check_size(w, h, 0, s)) ||
> >>>>>>>>>> + (!is_video && audio_check_size(s->channels, pic->nb_samples, s->sample_fmt)))
> >>>>>>>>>> return -1;
> >>>>>>>>>>
> >>>>>>>>>> if(s->internal_buffer==NULL){
> >>>>>>>>>> @@ -253,17 +277,30 @@ int avcodec_default_get_buffer(AVCodecContext *s, AVFrame *pic){
> >>>>>>>>>> picture_number= &(((InternalBuffer*)s->internal_buffer)[INTERNAL_BUFFER_SIZE]).last_pic_num; //FIXME ugly hack
> >>>>>>>>>> (*picture_number)++;
> >>>>>>>>>>
> >>>>>>>>>> - if(buf->base[0] && (buf->width != w || buf->height != h || buf->pix_fmt != s->pix_fmt)){
> >>>>>>>>>> + if (buf->base[0]) {
> >>>>>>>>>> + if (is_video && (buf->width != w || buf->height != h || buf->pix_fmt != s->pix_fmt)){
> >>>>>>>>>> for(i=0; i<4; i++){
> >>>>>>>>>> av_freep(&buf->base[i]);
> >>>>>>>>>> buf->data[i]= NULL;
> >>>>>>>>>> }
> >>>>>>>>>> + } else if (!is_video && (buf->channels != s->channels ||
> >>>>>>>>>> + buf->nb_samples != pic->nb_samples ||
> >>>>>>>>>> + buf->sample_fmt != s->sample_fmt)) {
> >>>>>>>>>> + if (buf->base[0] == s->user_buffer) {
> >>>>>>>>>> + s->user_buffer_in_use = 0;
> >>>>>>>>>> + buf->base[0] = NULL;
> >>>>>>>>>> + } else {
> >>>>>>>>>> + av_freep(&buf->base[0]);
> >>>>>>>>>> + }
> >>>>>>>>>> + buf->data[0] = NULL;
> >>>>>>>>>> + }
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> if(buf->base[0]){
> >>>>>>>>>> pic->age= *picture_number - buf->last_pic_num;
> >>>>>>>>>> buf->last_pic_num= *picture_number;
> >>>>>>>>>> }else{
> >>>>>>>>>> + if (is_video) {
> >>>>>>>>>> int h_chroma_shift, v_chroma_shift;
> >>>>>>>>>> int size[4] = {0};
> >>>>>>>>>> int tmpsize;
> >>>>>>>>>> @@ -327,6 +364,28 @@ int avcodec_default_get_buffer(AVCodecContext *s, AVFrame *pic){
> >>>>>>>>>> buf->height = s->height;
> >>>>>>>>>> buf->pix_fmt= s->pix_fmt;
> >>>>>>>>>> pic->age= 256*256*256*64;
> >>>>>>>>>> + } else { /* audio */
> >>>>>>>>>> + int buf_size;
> >>>>>>>>>> +
> >>>>>>>>>> + buf->last_pic_num = -256*256*256*64;
> >>>>>>>>>> +
> >>>>>>>>>> + buf_size = pic->nb_samples * s->channels *
> >>>>>>>>>> + (av_get_bits_per_sample_format(s->sample_fmt) / 8);
> >>>>>>>>>> +
> >>>>>>>>>> + if (s->user_buffer && !s->user_buffer_in_use && s->user_buffer_size >= buf_size) {
> >>>>>>>>>> + buf->base[0] = s->user_buffer;
> >>>>>>>>>> + s->user_buffer_in_use = 1;
> >>>>>>>>>> + } else {
> >>>>>>>>>> + buf->base[0] = av_mallocz(buf_size);
> >>>>>>>>>> + if (!buf->base[0])
> >>>>>>>>>> + return AVERROR(ENOMEM);
> >>>>>>>>>> + }
> >>>>>>>>>> +
> >>>>>>>>>> + buf->data[0] = buf->base[0];
> >>>>>>>>>> + buf->channels = s->channels;
> >>>>>>>>>> + buf->nb_samples = pic->nb_samples;
> >>>>>>>>>> + buf->sample_fmt = s->sample_fmt;
> >>>>>>>>>> + }
> >>>>>>>>>> }
> >>>>>>>>>> pic->type= FF_BUFFER_TYPE_INTERNAL;
> >>>>>>>>>>
> >>>>>>>>>> @@ -360,9 +419,15 @@ void avcodec_default_release_buffer(AVCodecContext *s, AVFrame *pic){
> >>>>>>>>>> }
> >>>>>>>>>> assert(i < s->internal_buffer_count);
> >>>>>>>>>> s->internal_buffer_count--;
> >>>>>>>>>> + if (buf->base[0] == s->user_buffer) {
> >>>>>>>>>> + assert(s->user_buffer_in_use);
> >>>>>>>>>> + s->user_buffer_in_use = 0;
> >>>>>>>>>> + buf->base[0] = NULL;
> >>>>>>>>>> + } else {
> >>>>>>>>>> last = &((InternalBuffer*)s->internal_buffer)[s->internal_buffer_count];
> >>>>>>>>>>
> >>>>>>>>>> FFSWAP(InternalBuffer, *buf, *last);
> >>>>>>>>>> + }
> >>>>>>>>>>
> >>>>>>>>>> for(i=0; i<4; i++){
> >>>>>>>>>> pic->data[i]=NULL;
> >>>>>>>>> i dont see how this could work
> >>>>>>>>> the buffer used and returned by the previous decode() is put in a que by the
> >>>>>>>>> user app and user_buffer is set to a new buffer.
> >>>>>>>>> also you appear to end up calling av_free()
> >>>>>>>>> on user supplied buffers
> >>>>>>>> Well, I meant to disallow that, but the documentation I put just says
> >>>>>>>> the user cannot free or change the data while user_buffer_in_use is set.
> >>>>>>>> I didn't consider the user replacing it with a new buffer. But at any
> >>>>>>>> rate, if that should be allowed, things get more complicated. I'll need
> >>>>>>>> to add a flag or something to indicate each user-supplied buffer. I'll
> >>>>>>>> work on it.
> >>>>>>> i think the API is too complex already and i dont see why it is so
> >>>>>>> if user buffer is set get_buffer() should return it or fail, if its returned
> >>>>>>> it should set user_buffer to NULL
> >>>>>>> calling get_buffer() a second time if user_buffer was set should be disallowed
> >>>>>>> release_buffer should do nothing
> >>>>>>>
> >>>>>>> if we ever have decoders that dont work with this then we need a AVCodec flag
> >>>>>>> that indicates them. For this case get_buffer() would then ignore user_buffer
> >>>>>>> and avcodec_decode() would copy to the provided user_buffer if any.
> >>>>>>> (we do not need this currently though because we do not have such a decoder)
> >>>>>>>
> >>>>>>> maybe iam missing something but this seems simpler
> >>>>>> Ok I did it a different way. New patch attached. I'm not 100% sure
> >>>>>> about the way reget_buffer() is handled but it works. AVFrame.type
> >>>>>> seems to be a mask, but I don't know if it was intended to be used that way.
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Justin
> >>>>>>
> >>>>>>
> >>>>>> doc/APIchanges | 9 ++
> >>>>>> libavcodec/avcodec.h | 100 ++++++++++++++++++++++++++++++--
> >>>>>> libavcodec/pcm.c | 41 +++++++++++--
> >>>>>> libavcodec/utils.c | 157 ++++++++++++++++++++++++++++++++++++++++++++-------
> >>>>>> 4 files changed, 275 insertions(+), 32 deletions(-)
> >>>>>> 39eb7fb791089c0822e3f87d3226b49131563a72 avcodec_decode_audio4.patch
> >>>>>> diff --git a/doc/APIchanges b/doc/APIchanges
> >>>>>> index 4155d32..a39d9fd 100644
> >>>>>> --- a/doc/APIchanges
> >>>>>> +++ b/doc/APIchanges
> >>>>>> @@ -13,6 +13,15 @@ libavutil: 2009-03-08
> >>>>>>
> >>>>>> API changes, most recent first:
> >>>>>>
> >>>>>> +2010-XX-XX - rXXXXX - lavc 52.92.0 - AVFrame and avcodec_decode_audio
> >>>>>> + Add nb_samples field to AVFrame.
> >>>>>> + Add user_buffer, user_buffer_size, and user_buffer_in_use fields to AVCodecContext.
> >>>>>> + Deprecate AVCODEC_MAX_AUDIO_FRAME_SIZE.
> >>>>>> + Deprecate avcodec_decode_audio3() in favor of avcodec_decode_audio4().
> >>>>>> + avcodec_decode_audio4() writes output samples to an AVFrame, which gives the
> >>>>>> + audio decoders the ability to use get/release/reget_buffer() to get an
> >>>>>> + output buffer.
> >>>>>> +
> >>>>>> 2010-10-10 - r25441 - lavfi 1.49.0 - AVFilterLink.time_base
> >>>>>> Add time_base field to AVFilterLink.
> >>>>>>
> >>>>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >>>>>> index 4bddbaa..1aa1c8c 100644
> >>>>>> --- a/libavcodec/avcodec.h
> >>>>>> +++ b/libavcodec/avcodec.h
> >>>>>> @@ -31,7 +31,7 @@
> >>>>>> #include "libavutil/cpu.h"
> >>>>>>
> >>>>>> #define LIBAVCODEC_VERSION_MAJOR 52
> >>>>>> -#define LIBAVCODEC_VERSION_MINOR 92
> >>>>>> +#define LIBAVCODEC_VERSION_MINOR 93
> >>>>>> #define LIBAVCODEC_VERSION_MICRO 0
> >>>>>>
> >>>>>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> >>>>>> @@ -467,8 +467,10 @@ enum SampleFormat {
> >>>>>> CH_FRONT_LEFT_OF_CENTER|CH_FRONT_RIGHT_OF_CENTER)
> >>>>>> #define CH_LAYOUT_STEREO_DOWNMIX (CH_STEREO_LEFT|CH_STEREO_RIGHT)
> >>>>>>
> >>>>>> +#if FF_API_AUDIO_OLD
> >>>>>> /* in bytes */
> >>>>>> #define AVCODEC_MAX_AUDIO_FRAME_SIZE 192000 // 1 second of 48khz 32bit audio
> >>>>>> +#endif
> >>>>>>
> >>>>>> /**
> >>>>>> * Required number of additionally allocated bytes at the end of the input bitstream for decoding.
> >>>>>> @@ -988,7 +990,13 @@ typedef struct AVPanScan{
> >>>>>> * - decoding: Set by libavcodec\
> >>>>>> */\
> >>>>>> void *hwaccel_picture_private;\
> >>>>>> -
> >>>>>> +\
> >>>>>> + /**\
> >>>>>> + * number of audio samples (per channel) described by this frame\
> >>>>>> + * - encoding: Set by user.\
> >>>>>> + * - decoding: Set by libavcodec.\
> >>>>>> + */\
> >>>>>> + int nb_samples;\
> >>>>>>
> >>>>>> #define FF_QSCALE_TYPE_MPEG1 0
> >>>>>> #define FF_QSCALE_TYPE_MPEG2 1
> >>>>>> @@ -2744,6 +2752,33 @@ typedef struct AVCodecContext {
> >>>>>> * - decoding: unused
> >>>>>> */
> >>>>>> int lpc_passes;
> >>>>>> +
> >>>>>> + /**
> >>>>>> + * User-allocated audio decoder output buffer & buffer size
> >>>>>> + * If user_buffer is non-NULL and is large enough,
> >>>>>> + * avcodec_default_get_buffer() may user it as an internal buffer instead
> >>>>>> + * of allocating its own. This only works with decoders that support
> >>>>>> + * CODEC_CAP_DR1. If the decoder uses this buffer, it will set the value
> >>>>>> + * to NULL.
> >>>>>> + *
> >>>>>> + * @note The user may not use this field when using avcodec_decode_audio3()
> >>>>>> + * or avcodec_decode_audio2().
> >>>>> I dont see why we need such special casing. *samples could be passed in the new
> >>>>> api like it is in the old
> >>>> The old API already has the samples buffer passed directly. Why should
> >>>> the old API be changed to accept user_buffer as an alternative to
> >>>> *samples? Do they have to match? If not, which takes priority? This
> >>>> would require added documentation to an old API. That seems more
> >>>> complexity than necessary when the ability to supply a direct buffer is
> >>>> already there.
> >>> you misunderstand
> >>> why is the new api having it passed over AVCodecContext and the old over an
> >>> function argument (in the sense why dont you change the new api to also take
> >>> a argument for that?) maybe i miss something but this seems to be simpler
> >> Ah, thanks for clarifying.
> >>
> >> The new API doesn't need it. The old API needs it in order to avoid
> >> memcpy. In the new API, if the user wants a direct buffer, she can
> >> override get/release_buffer().
> >
> > she can but that is more code and work
>
> Ok. I'm somewhat against it, but I'm willing to implement it.
can you elaborate on why you are against it?
to me this looks nicer and more logic from the outsides users point of view
>
> my preference:
>
> int avcodec_decode_audio4(AVCodecContext *avctx, AVFrame *frame,
> int *got_frame_ptr, AVPacket *avpkt,
> void *samples, int samples_size);
>
> What do you think?
i like it
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20101113/c7a00dd0/attachment.pgp>
More information about the ffmpeg-devel
mailing list