Hi, On 30.12.2015 02:09, Ronald S. Bultje wrote: > On Tue, Dec 29, 2015 at 6:54 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: >> I prefer that input for such performance-sensitive dsp code gets sanitized, >> so that overflows are (nearly) impossible. > > > I don't think it makes sense to make the decoder 5% slower just so that > ubsan is happy. We don't do that for other video decoders either. And yes, > the coefficient decoding loop is typically a major hotspot in video codecs.
It's not quite 5%, only 2-3%, but I agree that this slowdown is not acceptable. Maybe it's possible to do the input sanitizing earlier, outside of performance sensitive code. On 30.12.2015 03:12, Ganesh Ajjanagadde wrote: > 2. If things can't be transformed to be safe without leading to > significant loss of performance, such things must be clearly > documented as comments, fully justified, and very carefully > scrutinized IMHO. I agree. If the overflows can't be reasonably fixed, they should at least be documented. On 30.12.2015 04:13, Rostislav Pehlivanov wrote: > I agree with BBB (Ronald), this project claims to have the fastest decoders > available and we shouldn't sacrifice performance. Since basically the only > input of the decoder goes through the entropy decoding system occasionally > checking for overflows in a few performance sensitive parts there is worth > it, as it has been pointed out. If a few more checks in the entropy decoding system would prevent these overflows, that would be great. > Also I'd like to remind people that this is an RFC and WIP. Overflow > checking and fuzzing aren't high priority yet. Experience has shown that trying to fix such issues a few month after a decoder has been added is rather tedious. > I wanted to know if I was doing something horribly wrong, typos, naive > code, asserts/crashes on normal files, glitches, etc. None of that seems to be the case. ;) However, I think this decoder should be flagged with AV_CODEC_CAP_EXPERIMENTAL, at least until it can decode more than I-frames. One thing I find rather annoying: there are lots of quite long #define's in daaladsp.c, which is bad for debugging purposes, as the debugger treats each #define only as one line. Since most of these defines are used exactly once, I'm wondering why you don't use the code directly. I think that would also improve readability. Best regards, Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
