[FFmpeg-devel] [PATCH][WIP] lavfi/aconvert: use libswresample.

Clément Bœsch ubitux at gmail.com
Thu Jan 26 17:24:49 CET 2012


On Thu, Jan 26, 2012 at 04:05:32PM +0100, Nicolas George wrote:
> Le septidi 7 pluviôse, an CCXX, Clément Bœsch a écrit :
> > BTW, what is planned for the audio packing in lavfi?
> 
> I think I do not understand what you mean here.
> 

As you mentioned later, lavfi has a bit flag to indicates if it is
processing a planar or packed format. It was done like this before the
planar formats got introduced.

Now the information is "embedded" in the sample format and we don't need
that bit anymore, so that attribute could be dropped. It would also
simplifies of this filter by dropping the last parameter (you use "s16" if
you want packed signed 16-bit samples, and "s16p" if you want planar
signed 16-bit samples).

Still, dropping the packing format flag has various implications:
 - we won't have a way of specifying "auto" for the planar/packed if we
   don't "auto" the sample format
 - the packing format is an enum which might have become more than just
   "packed" or "planar"; dropping it might put an end to other potential
   way of packing samples (unlikely to reach the filters?)
 - ABI/API?

> > @@ -163,6 +82,9 @@ static int query_formats(AVFilterContext *ctx)
> >      avfilter_formats_ref(avfilter_make_all_formats(AVMEDIA_TYPE_AUDIO),
> >                           &inlink->out_formats);
> >      if (aconvert->out_sample_fmt != AV_SAMPLE_FMT_NONE) {
> > +        if (aconvert->out_packing_fmt != -1)
> > +            aconvert->out_sample_fmt = av_get_alt_sample_fmt(aconvert->out_sample_fmt,
> > +                                                             aconvert->out_packing_fmt);
> 
> I think there is something fishy I do not understand going on here. Correct
> me if I am wrong: libavfilter has both the AVFilterLink.planar bit and the
> information in the sample format (FOO42 of FOO42P), and filters that
> actually handle planar samples have to play along and hide that fact from
> the user.
> 

This part of the code is some kind of workaround to add (or remove) the
planar/packed flag from the sample format if the packing is specified,
(that information need to be in the sample format for libswresample if you
want it to be honored).

Maybe this is the thing I don't handle well and produces the crash I
mentioned earlier.

> >      av_get_channel_layout_string(buf1, sizeof(buf1),
> >                                   -1, inlink ->channel_layout);
> >      av_get_channel_layout_string(buf2, sizeof(buf2),
> >                                   -1, outlink->channel_layout);
> 
> Nit: in_layout_name_in and out_layout_name, maybe? But I realize after
> having written that that this is not part of the patch, this was the
> original code. Disregard this comment.
> 

I wanted to rename a few things now that I dropped most of the code, but
the diff is big enough right now, so I was willing to do it later
eventually.

[...]

-- 
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/20120126/5276d04d/attachment.asc>


More information about the ffmpeg-devel mailing list