On Sun, 27 Nov 2016, Nicolas George wrote:
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.
There can be filters where .query_format is defined and they still can
refer to all_formats and not all_counts, so i am not sure we can remove
the all_formats/all_counts logic so easily.
Anyway, how can we move forward here? Apparently there is not too much
interest in the topic... Since this can be easily changed later, I can
also rework the patch to add the .query_formats callback to all filters
currently not supporting unkown layouts until this is decided. What do you
think? Obviously I prefer my original approach, but since it is not too
much work, I can change the patch as well.
Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
