[FFmpeg-devel] [PATCH 1/3] lavc: do not init frame with guessed layout.
Hendrik Leppkes
h.leppkes at gmail.com
Thu Mar 14 23:45:58 CET 2013
On Thu, Mar 14, 2013 at 11:44 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> On Thu, Mar 14, 2013 at 11:33 PM, Nicolas George
> <nicolas.george at normalesup.org> wrote:
>> Le quartidi 24 ventôse, an CCXXI, Hendrik Leppkes a écrit :
>>> I just did a quick grep, and its used in very critical parts that
>>> would immediately break if you combine planar audio with unknown
>>> channel layout, because its used to allocate the planar data buffers.
>>>
>>> If frames with unknown channel layout are supposed to be supported,
>>> those cases should definitely be changed first to use
>>> AVFrame->channels. The fork doesn't have have channels and relys on
>>> channel_layout alone, which is why its used in all critical parts.
>>> Basically, all cases where av_get_channel_layout_nb_channels is used
>>> in avutil/frame.c and quite some in avfilter would be affected by
>>> this.
>>>
>>> So, before this is the case, i'm quite sure it would cause severe
>>> regressions, even if none appear in FATE.
>>
>> I do not think it can cause severe regressions, because they would only
>> happen in cases that currently, more or less, do not work. You can not
>> regress from "do now work".
>>
>
> Well look at avutil/frame.c, av_frame_get_buffer checks for the layout
> and otherwise errors, and get_audio_buffer uses it to determine the
> number of channels, and its used in many more places in that file to
> determine the number of channels for allocation or buffer
> copy.(av_frame_ref/av_frame_make_writable).
> And that file is very central for how avcodec and avfilter work.
>
> To be honest, reading this code, i'm not sure how it works with any
> file with unknown layout right now, but not sure if i actually have a
> file like that to check this.
And with right now, i mean with your patch, because right now in HEAD
it creates a dummy channel layout, of course.
More information about the ffmpeg-devel
mailing list