[FFmpeg-devel] [PATCH] lavfi/avfiltergraph: always reduce all_layouts to a single layout

[Nicolas George]

Le tridi 3 frimaire, an CCXXV, Marton Balint a écrit :
> I thought we are trying to move away from workarounds introduced only to be
> able to be compatible with libav API. So this is clearly libav dirving
> ffmpeg development, which is not fortunate IMHO.
> 
> I also think that the chance of an exploitable filter because of this is
> small. An audio filter with no query_formats is quite rare in itself. Even
> if such a filter got merged, making it work with unknown channel layouts is
> a feature which we would want anyway, becase ffmpeg does support unknown
> channel layouts.
> 
> Yes, it is not me who does the merges, but IMHO this does not add too much
> burden for the people who does it. Hendrik, Clement, what do you think?
> 
> And even if such an issue got in the codebase, isn't this something that
> coverity should be able to easily detect most of the times?

In principle, I think we would want to get rid of the work-ardounds and
have all filters support unknown layouts. But we also want our users to
have a working and reliable tool. Meeting both objectives requires
auditing and testing.

If we make unknown layouts the default and a filter that does not work
with them sneaks through, it will sometimes use 0 as the number of
channels. Depending on how much it does so, it can have several
unfortunate consequences, most probably either a generic error message
(probably "invalid argument" or "out of memory"), a corrupted output or
a crash (and unless proven otherwise, a crash must be assumed to be
exploitable).

Automated testing can not help us catching them. Neither FATE nor
coverity, the way they currently work.

On the other hand, detecting filters that do not work correctly is not
very hard in itself. The use of av_get_channel_layout_nb_channels() is a
very reliable condition. But it could be hidden in a call to an external
function. Making them work is usually rather easy too: replace
av_get_channel_layout_nb_channels() with the corresponding "channels"
field.

Here is, to the best of my knowledge, the current state of affairs. With
that information, we can decide to make unknown layouts the default. But
that is not my decision alone.

If it happens, I will try to help paying attention to new filters, but I
can not promise I will succeed catching them all.

Also, if we decide to proceed, I think we should not just make
all_counts the default: the all_counts / all_layouts logic is rather
complex, and with all_counts the default it becomes essentially dead
code. But the decision comes first.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161127/2dac6071/attachment.sig>