[FFmpeg-devel] [PATCH] Add af_resample - sample fmt and channel layout conversion filter

[S.N. Hemanth Meenakshisundaram]

On 08/23/2010 08:13 AM, Stefano Sabatini wrote:
> On date Monday 2010-08-23 00:11:52 -0700, S.N. Hemanth Meenakshisundaram encoded:
>   
>> Fixed the query formats function so that a resample filter can be
>> inserted between filters with incompatible sample formats.
>>
>>     
>> +
>> +/* Stereo to 5.1 output */
>> +static void ac3_5p1_mux(uint8_t *out[], uint8_t *in[], int samples_nb, int in_channels)
>> +{
>> +    int i;
>> +    short *output = (short *) out[0];
>> +    short *input = (short *) in[0];
>> +    short left, right;
>> +
>> +    for (i = 0; i < samples_nb; i++) {
>> +      left  = *input++;                 /* Grab next left sample */
>> +      right = *input++;                 /* Grab next right sample */
>> +      *output++ = left;                 /* left */
>> +      *output++ = right;                /* right */
>> +      *output++ = (left>>1)+(right>>1); /* center */
>> +      *output++ = 0;                    /* low freq */
>> +      *output++ = 0;                    /* FIXME: left surround is either -3dB, -6dB or -9dB of stereo left */
>> +      *output++ = 0;                    /* FIXME: right surroud is either -3dB, -6dB or -9dB of stereo right */
>> +    }
>> +}
>>     
> I suppose this code is taken from libavcodec, so there is no point
> into reviewing it. Maybe a better idea, rather than duplicate the
> code, could be to try to export this API as internal functions
> (ff_prefixed).
>
>   

The issue with the lavc code for channel mixing is that the functions
have varying number of arguments. So, depending on the channel
conversion, we would need to have a different call. Here all functions
operate on the same arguments.

Is it ok to have a switch case or if-else-if with different calls and
use the existing lavc functions?

>> +        sscanf(args, "%d:%ld", &resample->out_sample_fmt,
>> +               &resample->out_channel_layout);
>>     
> possibly it should take "%s:%s", sample_fmt <=> int mapping should be
> already supported by the API, as for the channel layout there is your
> patch waiting for review.
>  
>   

Sure. Will do once that patch is in/modified otherwise we would have one
string and one numeric argument now. I have a patch for this already.

> Possibly print a string here for the sample format.
> Same here, better to give a string representation.
>   

Both these print the sample format/channel layout only if they are not
in the table, so they would have no valid string representation.

>> [...]
>> +        for (sample_fmt = 0; sample_fmt < SAMPLE_FMT_NB; sample_fmt++)
>> +            if ((ret = avfilter_add_format(&formats, sample_fmt)) < 0) {
>> +                avfilter_formats_unref(&formats);
>> +                return ret;
>> +            }
>>     
> formats = avfilter_all_formats()
>   

Done.

> [...]
> why not to call av_audio_convert()?
>   

Done. This patch only works on top of the fix for
default_get_audio_buffer that I posted just before this.


> [...]
> Missing docs (not high priority, feel free to add it when we're
> approaching to the final version).
>   

Will add once we have a version closer to the final one.

> Regards.
>   

The posted patch can be tested with

./ffplay -af resample=<sample_fmt>:<channel_layout> in.ogg

Passes 'make test' and on top of SVN latest. No errors from valgrind.

Regards,

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-af_resample-sample-fmt-and-channel-layout-conver.patch
Type: text/x-patch
Size: 20352 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100825/d37f1617/attachment.bin>