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

[Stefano Sabatini]

On date Saturday 2011-01-15 20:18:20 +0100, Michael Niedermayer encoded:
> On Sat, Jan 15, 2011 at 07:24:01PM +0100, Stefano Sabatini wrote:
> > On date Saturday 2011-01-15 14:24:35 +0100, Michael Niedermayer encoded:
> > > On Sat, Jan 15, 2011 at 02:27:03AM +0100, Stefano Sabatini wrote:
> > > > ---
> > > >  libavcore/audioconvert.c |   14 ++++++++++++++
> > > >  libavcore/audioconvert.h |   21 +++++++++++++++++++++
> > > >  2 files changed, 35 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/libavcore/audioconvert.c b/libavcore/audioconvert.c
> > > > index 1c6a511..7a46a3e 100644
> > > > --- a/libavcore/audioconvert.c
> > > > +++ b/libavcore/audioconvert.c
> > > > @@ -166,3 +166,17 @@ int av_samples_fill_pointers(uint8_t *pointers[8], uint8_t *buf, int buf_size,
> > > >  
> > > >      return per_channel_size / sample_size;
> > > >  }
> > > > +
> > > > +int av_samples_fill_arrays(uint8_t *data[8], int linesizes[8],
> > > > +                           uint8_t *buf, int buf_size,
> > > > +                           enum AVSampleFormat sample_fmt, int planar,
> > > > +                           int64_t channel_layout, int nb_channels)
> > > > +{
> > > 
> > > again channel_layout has nothing to do with what the code does
> > 
> > Of course, so now I wonder if libavcore/samples.h is a better place
> > for this API.
> > 
> > Now it is:
> > int av_samples_fill_linesizes(int linesizes[8], enum AVSampleFormat sample_fmt,
> >                               int planar, int nb_channels);
> > int av_samples_fill_pointers(uint8_t *pointers[8], uint8_t *buf, int buf_size,
> >                              enum AVSampleFormat sample_fmt, int planar,
> >                              int nb_channels);
> > 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);
> > 
> > This is useful for factorizing code in
> > avfilter_default_get_audio_buffer() (see attached patch) and in
> > avfilter_get_audio_buffer_ref_from_buffer().
> > 
> > I can eventually merge the implementation of the three functions in 
> > av_samples_fill_arrays(), since currently there is not the need of two
> > separate av_samples_fill_linesizes/pointers functions.
> > 
> > Please comment on this API.
> 
> I agree with avcore
> 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?

Check new patch.
-- 
FFmpeg = Fantastic and Fierce Majestic Picky Educated Governor