[FFmpeg-devel] [RFC] libavfilter audio - af_resample

Michael Niedermayer michaelni
Sun Jul 11 00:30:04 CEST 2010


On Sat, Jul 10, 2010 at 12:05:43PM +0100, M?ns Rullg?rd wrote:
> "S.N. Hemanth Meenakshisundaram" <smeenaks at ucsd.edu> writes:
> 
> > Michael Niedermayer wrote:
> >> On Wed, Jul 07, 2010 at 04:31:04AM -0700, S.N. Hemanth Meenakshisundaram wrote:
> >>
> >>> On 07/04/2010 07:57 AM, Stefano Sabatini wrote:
> >>>
> >>>> On date Thursday 2010-07-01 01:42:51 -0700, S.N. Hemanth
> >>>> Meenakshisundaram encoded:
> >>>>
> >>>>> On 06/25/2010 05:10 PM, Stefano Sabatini wrote:
> >>>>>
> >>>>>> On date Friday 2010-06-25 03:52:45 -0700, S.N. Hemanth
> >>>>>> Meenakshisundaram encoded:
> >>>>>>
> >>>>> Hi All,
> >>>>>
> >>>>> Here is the working af_resample.c [...]
> >>
> > Hi All,
> >
> > I am attaching a reworked af_resample.c
> > The following are the major changes:
> >
> > 1. Samples are now copied only when a conversion (of channels or
> > sample format) is needed, else the same buffer is passed along.
> 
> That's not what I call copying.  I hope you're not copying the samples
> and then converting them.

I hope you are not flaming without reading the patch


> 
> > 2. Earlier, the channel conversion functions only worked for short
> > (SAMPLE_FMT_S16) pointers but writing a different version for each
> > will increase the number of functions required by 5x. To get around
> > this, I have chosen to always convert incoming audio data to
> > SAMPLE_FMT_S16. Most of the times, this conversion is not necessary
> > since most codecs output S16 data. Once we ensure data is in S16
> > format, channel downmix/upmix is done and finally this data is
> > converted to required output sample format (in case it is not
> > SAMPLE_FMT_S16).
> 
> Entirely unacceptable.

rude comment and entirely unproductive
Hemanth is actually doing quite good work having 
nb_sample_fmts^2 * nb_channel_layouts^2 pieces of code is not practical.
having just s16 is also not ideal
i would suggest we support s16->s16 and float->float mixers and than a
full set of X->Y sample format converters to connect to them
sample formats different from s16 and float are rater rare so this should
be ok


> 
> > Although this seems wasteful, most of the times there should be only
> > one or zero sample format conversions since at least one of codec
> > output or required SDL output is likely to be S16. Please let me know
> > what you think of this.
> 
> Who cares what SDL does or doesn't do?

noone does, but then who told the student that we dont care about SDL?
i guess thats also noone, so he couldnt have known that

Fact is, we hate SDL, ffplay uses it because of historic reasons and its
planned to support other ways to get audio and video out from ffplay but
thats probably not going to happen soon


> 
> > The a52dec library for instance has the sample format fixed at compile
> > time and hence has only one version of all channel downmix/upmix
> > conversion routines.
> 
> liba52 is dead.  Forget what it used to do.  There's a reason it died.

id prefer if you could maybe spare our students of your random rants.
the downmix/upmix code previously submitted used large flat matrixes and
i pointed Hemanth to liba52 as it manages up/downmix without such tables
afaik. Its surely possible to represent the sparse matrixes more compactly
by not storing the 0 elements, this also could improve speed and we could
have specific functions for *mix() of common cases.


> 
> > 3. For the channel conversion, I have changed all routines to work
> > only for packed format for the time being. Again this is because,
> > there seems to be no way sending planar data to SDL audio and ffplay
> > also currently assumes data from codecs is in packed format. However,
> > support for planar data only requires additional channel xonversion
> > routines and no change to the af_resample framework.
> 
> The first thing we must do is add support for planar audio.

planar audio is important, but when its done is the decission of who works
on it, there is no point in droping everything and trying to add planar
first.


> 
> > 4. There is now a 1. existing stereo to mono, 2. existing mono to
> > stereo, 3. generic mono downmix that uses only 2 of the incoming
> > channels, 4. generic stereo downmix that uses only 2 of the incoming
> > channels and 5. the existing stereo to ac3 (5.1) converters.
> >
> > Please review and comment. The A52 library seems to have the required
> > formula for a lot of downmix/upmix fuunctions and it also uses
> > separate routines for different channel conversions. I also plan to
> > add additional channel conversion routines using mixing formulae from
> > this and other sources.
> > All downmix/upmix routines will have the same return type and argument
> > list and a function pointer in the context gets initialized to the
> > required routine whenever input channels change.
> 
> I dislike the idea of having basic mixing functionality only available
> as a full-blown filter.

i do like having just a single api through which any mix+sample convertion
can be requested and then the best and fastest implementation for this
is automatically selected

The reason why i like this, is that a single simple API is easy to document
remember and use. Also its very easy to export the subparts individually
at a later point if there is anyone wanting that.

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100711/3155a191/attachment.pgp>



More information about the ffmpeg-devel mailing list