[FFmpeg-devel] [PATCH]levc/hevc_cabac Optimise ff_hevc_hls_residual_coding (especially ARM)
jc at kynesim.co.uk
Tue Jan 19 17:46:31 CET 2016
>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 -
>> OK - not all:
>> 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 :-)
>ffmpeg-devel mailing list
>ffmpeg-devel at ffmpeg.org
More information about the ffmpeg-devel