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

[Stefano Sabatini]

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.
-- 
FFmpeg = Furious Furious Mastering Ponderous Elected Gospel