>John Cox <jc <at> kynesim.co.uk> writes: > >> On Tue, 19 Jan 2016 15:59:39 +0000 (UTC), you wrote: >> >> >John Cox <jc <at> kynesim.co.uk> writes: >> > >> >> >> +#define UNCHECKED_BITSTREAM_READER 1 >> >> > >> >> >I don't think that's right, and is a security issue. >> >> >> >> I added that line as (nearly) every other decoder in >> >> liavcodec has it - >> > >> >Sure? >> >> OK - not all: >> >> h263dec.c >> h264.c >> h264_cabac.c >> h264_cavlc.c >> huffyuvdec.c >> ituh263dec.c >> mpegl2dec.c >> mpeg12.c >> mpeg4videodec.c >> mpeg4video_parser.c >> >> But that probably covers 90% of the video streams >> decoded with ffmpeg > >The three decoders mpegvideo, h263/asp and h264 are >not "(nearly) every other decoder", sorry!
Sorry - I (obviously) misremembered the number of hits I got when I last did that search. >> >> in particular h264_cabac.c has it. >> > >> >Extensive testing was done before it was added. >> >> Testing that it doesn't seg-fault no matter what the >> input or some other sort of testing? > >Yes, tests that show that fuzzed input does not crash >the decoder are needed. > >But afaict, the change is unrelated to the rest of your >patch and should be discussed separately (imo). Yup - perfectly happy to put that can of worms to one side. >> >Could you confirm how much of the speedup comes >> >only from this change? >> >> Not an awful lot - a few % of the total improvement, but >> I was looking for everything I can get. I'll happily >> take it out of this patch if it is controversial. > >I wouldn't say controversial (I am all for it, sorry if >this wasn't clear) but I think it can be discussed after >your speedup was committed. Yup - at this point it is simply a distraction >> >While we definitely all welcome a noticeable speedup >> >of hevc decoding (and while my opinion on your patch >> >has limited relevance) I believe that the patch >> >absolutely has to be split: First step would be to >> >have a split between changes in the general code and >> >changes to arm assembly, I believe the first patch >> >then may be split further. >> >> Happy to split out the arm asm. > >Please do, my suggestion would be to start with the >changes to the C code. But it may be wise to wait for a >real review first. I've done enough review processes to know that waiting till the comments die down before doing anything is the way to go :-) JC >Carl Eugen > >_______________________________________________ >ffmpeg-devel mailing list >ffmpeg-devel@ffmpeg.org >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
