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

[Stefano Sabatini]

On date Tuesday 2011-02-01 16:59:08 +0100, Stefano Sabatini encoded:
> 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
> 
> 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.

Patch updated with a fix.
-- 
FFmpeg = Fiendish and Foolish Mastering Powered Encoding/decoding Guru