[FFmpeg-devel] [PATCH] Implement av_samples_fill_arrays().
Michael Niedermayer
michaelni
Fri Jan 28 21:13:30 CET 2011
On Fri, Jan 28, 2011 at 12:55:09AM +0100, Stefano Sabatini wrote:
> 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?
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
>
> Check new patch.
> --
> FFmpeg = Fantastic and Fierce Majestic Picky Educated Governor
> samplefmt.c | 33 +++++++++++++++++++++++++++++++++
> samplefmt.h | 29 +++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
> 1b8852e158f3f1efa3811e230f94aa503c506e9c 0003-Implement-av_samples_fill_arrays.patch
> From 2d8bb08f6bea315948af9cbc6c37964e3ba54930 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Sat, 15 Jan 2011 00:00:00 +0100
> Subject: [PATCH] Implement av_samples_fill_arrays().
>
> ---
> libavcore/samplefmt.c | 33 +++++++++++++++++++++++++++++++++
> libavcore/samplefmt.h | 29 +++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+), 0 deletions(-)
>
> diff --git a/libavcore/samplefmt.c b/libavcore/samplefmt.c
> index 532acd9..d2a7c4f 100644
> --- a/libavcore/samplefmt.c
> +++ b/libavcore/samplefmt.c
> @@ -68,3 +68,36 @@ int av_get_bits_per_sample_fmt(enum AVSampleFormat sample_fmt)
> return sample_fmt < 0 || sample_fmt >= AV_SAMPLE_FMT_NB ?
> 0 : sample_fmt_info[sample_fmt].bits;
> }
> +
> +int av_samples_fill_arrays(uint8_t *pointers[8], int linesizes[8],
> + uint8_t *buf, int buf_size,
> + enum AVSampleFormat sample_fmt, int planar,
> + int nb_channels)
> +{
> + int i, step_size = 0;
> + int sample_size = av_get_bits_per_sample_fmt(sample_fmt) >>3;
> + int per_channel_size;
> +
> + per_channel_size = buf_size / nb_channels;
> + if (pointers) {
> + pointers[0] = buf;
> + if (planar) {
> + for (i = 1; i < nb_channels; i++) {
> + step_size += per_channel_size;
> + pointers[i] = buf + step_size;
> + }
> + } else { /* packed */
> + for (i = 1; i < nb_channels; i++)
> + pointers[i] = buf;
> + }
for (i = 0; i < nb_channels; i++) {
pointers[i] = buf + step_size;
if (planar)
step_size += per_channel_size;
}
> + memset(&pointers[nb_channels], 0, (8-nb_channels) * sizeof(pointers[0]));
> + }
> +
> + if (linesizes) {
> + for (i = 0; i < nb_channels; i++)
> + linesizes[i] = planar ? sample_size : nb_channels * sample_size;
> + memset(&linesizes[nb_channels], 0, (8-nb_channels) * sizeof(linesizes[0]));
> + }
> +
> + return per_channel_size / sample_size;
> +}
> diff --git a/libavcore/samplefmt.h b/libavcore/samplefmt.h
> index 9701efe..b1abe3d 100644
> --- a/libavcore/samplefmt.h
> +++ b/libavcore/samplefmt.h
> @@ -69,4 +69,33 @@ char *av_get_sample_fmt_string(char *buf, int buf_size, enum AVSampleFormat samp
> */
> int av_get_bits_per_sample_fmt(enum AVSampleFormat sample_fmt);
>
> +/**
> + * Fill channel data pointers and linesizes for samples with sample
> + * format sample_fmt.
> + *
> + * The data array is filled with the pointers to the samples data:
> + * for planar, set the start point of each channel's data within the buffer,
> + * for packed, set the start point of the entire buffer only.
iam not sure if this is a good idea, why does it differ from planar?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- 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/20110128/636c2eea/attachment.pgp>
More information about the ffmpeg-devel
mailing list