[FFmpeg-devel] [PATCH] Implement av_samples_fill_arrays().

Michael Niedermayer michaelni
Tue Feb 1 23:26:20 CET 2011


On Tue, Feb 01, 2011 at 04:59:08PM +0100, Stefano Sabatini wrote:
> On date Friday 2011-01-28 21:13:30 +0100, Michael Niedermayer encoded:
> > On Fri, Jan 28, 2011 at 12:55:09AM +0100, Stefano Sabatini wrote:
> [...]
> > > > what iam unsure about is linesize.
> > > > currently its all set equal, that is mightly useless
> > > > linesize[0] is as good as linesize[chan]
> > > > 
> > > > we could do something more usefull with linesize[1+]
> > > > like setting it so that
> > > > linesize[1] = data[1][0] - data[0][0]
> > > > 
> > > > the idea is that things could be addressed like
> > > > data[0] + time*linesize[0] +  chan*linesize[1]
> > > > the advantage of this over
> > > > data[chan] + time*linesize[0]
> > > > is that you need fewer variables (no data[0], data[1], data[2] ...) which can
> > > > help register starved architectures
> > > > 
> > > > 
> > > > also, if possible a single public function like:
> > > >  int av_samples_fill_arrays(uint8_t *data[8], int linesizes[8],
> > > >                             uint8_t *buf, int buf_size,
> > > >                             enum AVSampleFormat sample_fmt, int planar,
> > > >                             int nb_channels);
> > > >
> > > > that allows data & linesize to be NULL
> > > > would mean simpler public API thanb having 3 functions
> > > 
> > > Done, indeed it's much nicer.
> > > 
> > > As for the linesize stuff, I'm wondering if it is better to have
> > > something more similar to the imgutils stuff, that is to have linesize
> > > simply give the planar size (for planar) and the buffer size for
> > > packed (eventually aligned).
> > > 
> > > This would be useful for implementing:
> > > av_samples_copy
> > > av_samples_alloc
> > > 
> > > does it make sense?
> > 
> > you have 8 linesize values you set them all equal. you could store differnet
> > things in there instead of choosing which of 2 usefull values to duplicate 7
> > times
> 
> Updated again, implementing the idea from Michael, other patches
> attached for reference.
> 
> Note that we never defined the semantics of linesizes[] in the
> AVFilterBuffer for audio samples, the most natural approach is to
> assume the same semantics used by av_samples_*.
> 
> At this point some change is required (in the aconvert) because
> av_audio_convert() assumes a different semantics for in/out_stride:
> 
>  * Convert between audio sample formats
>  * @param[in] out array of output buffers for each channel. set to NULL to ignore processing of the given channel.
>  * @param[in] out_stride distance between consecutive output samples (measured in bytes)
>  * @param[in] in array of input buffers for each channel
>  * @param[in] in_stride distance between consecutive input samples (measured in bytes)
>  * @param len length of audio frame size (measured in samples)
>  */
> int av_audio_convert(AVAudioConvert *ctx,
>                            void * const out[6], const int out_stride[6],
>                      const void * const  in[6], const int  in_stride[6], int len);
> 
> Solutions:
> * create a wrapper in aconvert for converting samplesref->linesize to
>   in/out_stride semantics.
> * create an av_audio_convert2() supporting the new semantics

av_audio_convert() could just use linesize[0] and the API updated accordingly
i think 


> 
> It was discussed some months ago an avcodec_audio_decodeX() which was
> returning a packet rather than buffer+size as the current API, I
> suppose that may be relevant for this discussion as well.
> -- 
> FFmpeg = Furious Furious Mastering Ponderous Elected Gospel

[...]

> @@ -490,14 +490,15 @@ void avfilter_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref)
>  
>          link->cur_buf = avfilter_default_get_audio_buffer(link, dst->min_perms,
>                                                            samplesref->format,
> -                                                          samplesref->audio->size,
> +                                                          samplesref->audio->nb_samples,
>                                                            samplesref->audio->channel_layout,
>                                                            samplesref->audio->planar);
>          link->cur_buf->pts                = samplesref->pts;
>          link->cur_buf->audio->sample_rate = samplesref->audio->sample_rate;
>  
>          /* Copy actual data into new samples buffer */
> -        memcpy(link->cur_buf->data[0], samplesref->data[0], samplesref->audio->size);
> +        for (i = 0; samplesref->data[i]; i++)

some FF_ARRAY_ELEMS() check missing i suspect


[...]
>  avfilter.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  avfilter.h |   19 +++++++++++++++++++
>  2 files changed, 68 insertions(+)
> cddc1cb6f7d5135c0e58f690922e516ccdb8389e  0005-Implement-avfilter_get_audio_buffer_ref_from_arrays.patch
> From e51d3e60249ed43d8982bdbeb436b9960aff5cc5 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Mon, 31 Jan 2011 00:07:41 +0100
> Subject: [PATCH] Implement avfilter_get_audio_buffer_ref_from_arrays().
> 
> ---
>  libavfilter/avfilter.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/avfilter.h |   19 ++++++++++++++++++
>  2 files changed, 68 insertions(+), 0 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index d52d555..31af890 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -350,6 +350,55 @@ AVFilterBufferRef *avfilter_get_audio_buffer(AVFilterLink *link, int perms,
>      return ret;
>  }
>  
> +AVFilterBufferRef *
> +avfilter_get_audio_buffer_ref_from_arrays(uint8_t *data[8], int linesize[8], int perms,
> +                                          int nb_samples, enum AVSampleFormat sample_fmt,
> +                                          int planar, int64_t channel_layout,
> +                                          int nb_channels)
> +{
> +    AVFilterBuffer *samples = av_mallocz(sizeof(AVFilterBuffer));
> +    AVFilterBufferRef *samplesref = av_mallocz(sizeof(AVFilterBufferRef));
> +
> +    if (nb_channels < 0) {
> +        if (channel_layout < 0)
> +            goto fail;
> +        nb_channels = av_get_channel_layout_nb_channels(channel_layout);
> +    }
> +
> +    if (!samples || !samplesref)
> +        goto fail;
> +
> +    samplesref->buf = samples;
> +    samplesref->buf->free = ff_avfilter_default_free_buffer;
> +    if (!(samplesref->audio = av_mallocz(sizeof(AVFilterBufferRefAudioProps))))
> +        goto fail;
> +
> +    samplesref->audio->nb_samples     = nb_samples;
> +    samplesref->audio->channel_layout = channel_layout;
> +    samplesref->audio->planar         = planar;
> +
> +    /* make sure the buffer gets read permission or it's useless for output */
> +    samplesref->perms = perms | AV_PERM_READ;
> +
> +    samples->refcount = 1;
> +    samplesref->type = AVMEDIA_TYPE_AUDIO;
> +    samplesref->format = sample_fmt;
> +
> +    memcpy(samples->data,        data,     sizeof(samples->data));
> +    memcpy(samples->linesize,    linesize, sizeof(samples->linesize));
> +    memcpy(samplesref->data,     data,     sizeof(samplesref->data));
> +    memcpy(samplesref->linesize, linesize, sizeof(samplesref->linesize));
> +
> +    return samplesref;
> +
> +fail:
> +    if (samplesref && samplesref->audio)
> +        av_free(samplesref->audio);
> +    av_free(samplesref);
> +    av_free(samples);
> +    return NULL;
> +}
> +
>  int avfilter_request_frame(AVFilterLink *link)
>  {
>      FF_DPRINTF_START(NULL, request_frame); ff_dlog_link(NULL, link, 1);
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 22f8ff0..52da5d3 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -685,6 +685,25 @@ AVFilterBufferRef *avfilter_get_audio_buffer(AVFilterLink *link, int perms,
>                                               int64_t channel_layout, int planar);
>  
>  /**
> + * Create an audio buffer reference wrapped around an already
> + * allocated samples buffer.
> + *
> + * @param data pointers to the samples plane buffers
> + * @param linesize linesize for the samples plane buffers
> + * @param perms the required access permissions
> + * @param nb_samples number of samples per channel
> + * @param sample_fmt     the format of each sample in the buffer to allocate
> + * @param planar         audio data layout - planar or packed
> + * @param channel_layout the channel layout of the buffer, <=0 if unknown
> + * @param nb_channels the number of channels, <= 0 if unknown

this should list what is input and output


>  defaults.c |   48 ++++++++++++++----------------------------------
>  1 file changed, 14 insertions(+), 34 deletions(-)
> 3b6c64b8acb657face3ceddcd33d659f34f14056  0006-Use-avfilter_get_audio_buffer_ref_from_arrays-in.patch
> From 5679797f885c11889552dff546b418d80f412305 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Tue, 1 Feb 2011 16:45:12 +0100
> Subject: [PATCH] Use avfilter_get_audio_buffer_ref_from_arrays() in
>  avfilter_default_get_audio_buffer().

probably ok


also the alignment that is passed should be 16 not 15, 15 is odd for the API
to achive 16byte alignment

[...]
-- 
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/20110201/fb98c582/attachment.pgp>



More information about the ffmpeg-devel mailing list