[FFmpeg-devel] [PATCH 1/2] avcodec: Add AV_CODEC_FLAG2_FAST_UNSAFE, move unsafe uses of FAST to it

Michael Niedermayer michael at niedermayer.cc
Thu May 28 22:13:48 EEST 2020


On Thu, May 28, 2020 at 08:30:16PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-05-28 20:09:15)
> > 
> > h264 is a specific use of this flag, and that might not be the only
> > place it could be used in
> > 
> > But about h264 What this is about if i remember it correctly, is
> > that the maximum input any crafted bitstream of a block can require is X,
> > now you can if the input size is less than X copy that to a larger buffer or
> > you can add lots of checks. Both of these slow the code down a bit.
> > OTOH, if the stream is known to be valid that can be skipped.
> > 
> > It can also be skiped if the buffer is already big enough to begin with
> > OR if the output goes to the parser and not the decoder.
> > So even without the user having access to this, the codepath does not
> > become unneeded
> > the h264 case is more a "even if you cant proof its safe on case 123
> > use it anyway"
> > And quite possibly we can add more code detecting more cases where
> > it is safe, this should be investigated either way probably.
> 
> It does not seem to me that there is a sufficient use case for "decode
> as fast as possible, even at the cost of crashing sometimes". Big
> transcoders like youtube have process untrusted input and therefore care
> about correctness. End-user playback is either fast enough or
> hardware-accelerated (and thus fast enough).
> 
> What kind of users do you believe warrants this extra complexity?

The patch really was a response to kieran and jbs comments
They asked this to be either fixed or warnings to be added / documented

declaring the fast flag as unsafe would make it unusable for many
usecases.
And the specific feature can be removed / split to a different flag
but not really "fixed".
So this patch was born which moved it into a clearly documented new
flag.
Apparently that struck a nerve of some people, even though i asked
about it before posting it.

We can drop this patch and disable the one case, if thats what people
want

More generally though (outside this unsafe flag case) 
i do disagree with your argument a bit, performance does matter.
Iam regularly reminded of that for example, so much software becomes
slower on each upgrade with few if any features added the real users
care about. Just to pick one, the editor i use to write replies in mutt
is slower to close than before i upgraded the OS. 

Also again to stay general here, this does not apply to the unsafe flag.
speed / cpu load does add up. Slower code means more CO2 emissions if
the software is used alot.
If you want a real example insetad of this flag where we could improve
IIRC there was some code iterating over options in the iteration over
options resulting in some sort of O(n^3) or so. Thats from memory though
would need to check where that was exactly but thats something we should
fix. 

Thanks


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200528/e1b2b2b6/attachment.sig>


More information about the ffmpeg-devel mailing list