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

[Michael Niedermayer]

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


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

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"
-------------- 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/20110115/a82fcf38/attachment.pgp>