[FFmpeg-devel] [PATCH] lavfi: port MPlayer's af_pan filter to libavfilter.

[Clément Bœsch]

On Sat, Nov 05, 2011 at 03:26:12PM +0100, Nicolas George wrote:
> Le quintidi 15 brumaire, an CCXX, Clément Bœsch a écrit :
> > I wasn't able to contact the original author for a relicensing (I tried 5 or 6
> > mails I could find, but none seemed valid), so I kept the GPL and the original
> > (invalid) mail. It's OK with me to relicense to any preferred license if you
> > are able to contact Anders Johansson and find an agreement with him.
> 
> It looks that there are only a few lines of quite trivial code kept from the
> original. Maybe just reimplementing would be better?
> 

The "only" thing I kept is the panning code (with almost everything renamed):

    while (in < in_end) {
        for (o = 0; o < nb_output_channels; o++) {
            register float x = 0.0;
            for (i = 0; i < nb_input_channels; i++)
                x += in[i] * pan->gain_level[o][i];
            *out++ = x;
        }
        in += nb_input_channels;
    }

But since it's the core of the filter, I'd like to keep the original authorship.

> > +    float gain_level[SWR_CH_MAX][SWR_CH_MAX];
> 
> Since the rest of the code uses int16_t, maybe using integers for the
> coefficients too would be better? I believe that +7.8 fixed-point
> coefficients would be enough for any use.
> 

What if the filter is extended to support other sample formats?

> > +    // first parameter is the number of channels...
> > +    nb_output_channels = strtol(args, NULL, 10);
> > +    pan->out_channels_layout = av_get_default_channel_layout(nb_output_channels);
> 
> MPlayer has a fixed channel layout, but FFmpeg does not. Thus, I believe
> that the user should be able to specify the channel layout.
> 
> Possibly, av_get_channel_layout could be extended to accept fancy notations
> like "stereo+LFE+TBC" and also a single number.
> 
> (I am willing to do that, but I will be rather busy in the next ten days or
> so.)
> 

Feel free to update that patch when you feel like it then :)

Do you think this feature is blocking?

> > +    int nb_input_channels = av_get_channel_layout_nb_channels(inlink->channel_layout);
> > +    int nb_output_channels = av_get_channel_layout_nb_channels(pan->out_channels_layout);
> 
> Couldn't they be kept in the context structure rather than recomputed for
> each call?
> 

I'm not sure this really is a bottleneck, and it simplifies the declarations.
I'll resend a patch changing this in a moment anyway.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111105/17d16f70/attachment.asc>