[FFmpeg-devel] [GSoC] Qualification Task

[Peter Ross]

On Tue, Mar 22, 2011 at 01:32:42PM +0200, Mina Nagy Zaki wrote:
> On Tuesday 22 March 2011 12:28:10 Peter Ross wrote:
> > On Tue, Mar 22, 2011 at 09:54:02AM +0200, Mina Nagy Zaki wrote:
> > > Hi,

Hello

> > > I've ported a small filter from sox, the 'earwax' filter, essentially a
> > > stereo widening effect. It's a relatively simple filter, I was actually
> > > looking to write a general sox wrapper but turns out there would be a
> > > few things that would make this slightly difficult.
> > 
> > Can you elaborate on this, just curious.
> 
> There are two options: 
> 1. Allow for specification of multiple effects and create a sox effect chain. I 
> will not be able to use the public api as sox goes through an init stage then 
> sox_flow_effects() passes all the samples through from input to output. I would 
> have to feed samples into individual effects, in essence rewriting (or adapting 
> :) the sox_flow_effects() and helper functions it uses. Otherwise, a blocking 
> thread a custom input effect could be used.
> 2. Allow only one sox effect per libavfi effect.

That's one way to do it, and yes there would be those limitations.

Another way would be to use the libsox internal API. Each sox filter module exports
a symbol, e.g. 'lsx_earwax_effect_fn', from which you can then initialise and invoke
the filter directly. An audio filter wrapper like this would perform similar to the
MPlayer video filter wrapper.

[I am not suggesting you do this, just saying its technically possible.]

> In both cases I can't seem to figure out how I can find the end of stream, which 
> is important for some effects as they need to 'drain' (like the echo effect). 
> This actually leads to a larger discussion of the libavfi audio api in general. 
> We have the filter_samples() callback specifically for audio effects, while video 
> uses the request_frame() callback. Audio filters get samples 'pushed' to them, 
> while video filters AIUI pull frames from previous filters (note: I haven't 
> examined video filtering well). Moreover, request_frame() is actually used by 
> the audio src plugin which then initiates the filter_samples() reaction.
> 
> IMHO a unified API that pulls samples rather than pushes samples would be 
> better. So each effect can decide when to drain whenever it wishes (or when it 
> requests samples and doesn't get any). Sox uses something similar to the 
> current ffmpeg setup, pushing samples through one callback then calling a 
> different callback for draining.
> 
> Another major problem is what you asked at the very end: more than two 
> channels in the widen effect. I was actually planning to send a separate email 
> about this: Channel Negotiation. Not all effects can handle any number of 
> channels, and there's no way to do anything about it (except fail loudly) in 
> the current API. Again looking at sox, it does this by having effects declare 
> whether or not they can handle multiple channels. If they can then it simply 
> sends in the entire stream. If they can't then it creates an effect instance 
> for each channel, splits the stream, and 'flows' samples of one channel into 
> each of the effect instances. IMHO we _could_ improve on this.

Yep, a negotiation method is needed.

> [..]
> > Please include a license header at the top of the file. libsox is LGPL.
> Done.
> 
> > Do you use anything for audioconvert?
> Used to, not anymore. Removed.
> 
> > move the NUMTAPS define up higher and use 'static const int8_t
> > filt[NUMTAPS] ..'
> Done 
> 
> > > +/* 30??  330?? */
> >
> > those question marks should say 'degrees'
> They do actually! It's in unicode though.
> 
> > why not cut and paste the ASCII art from the sox file?
> Not sure why I removed it, but I put it back.
> 
> > the init() function is not neccessary, as libavfilter zeroises the priv
> > buffer for your automatically.
> Removed.
> 
> > might be more obvious to use 'endin = taps + NUMTAPS'
> IMHO 'in +' is more obvious as this just means endin points somewhere after 
> in, which it does.
> 
> > also pleae align, such that the equals signs line up. e.g.
> Aligned several places.
> 
> > the above two while loops are identical
> Yes, so an inline function or a define?

An inline function will be sufficient.

> > they also look a lot like scalarproduct_int16(), which can be found in
> > DSPContext
> I have to look into this, also regarding what Stefano mentioned.

Actually.. disregard what I said! As Stefano pointed out, DSPContext is part
of libavcodec. The task of making the dsp functions accessible to libavfilter
is can be done by somebody else. It is more important that you focus your
efforts on the audio filter system, IMHO. You can always revisit later if
you get the time.

Just place a //FIXME: replace with DSPContext.scalarproduct_int16 at 
the top of the inline function.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110322/acf23e5c/attachment.asc>