[FFmpeg-devel] [PATCH]levc/hevc_cabac Optimise ff_hevc_hls_residual_coding (especially ARM)

[John Cox]

Hi

>On Tue, Jan 19, 2016 at 7:46 AM, John Cox <jc at kynesim.co.uk> wrote:
>
>> Hi
>>
>> I've just done a fair bit of work on hevc_cabac decode for the Rasberry
>> Pi2 and I think that the patch is generally applicable.  Patch is
>> attached but you may prefer to take it from git:
>
>
>Cool! Two non-technical comments first, I'll try to make time to review
>in-depth/technically soon:
>
>1:
>
>> +#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 -
in particular h264_cabac.c has it.

Going forward: Checking bitstream position on every load is terribly
wasteful - if at all possible a better idea is to allocate more space
than is required in the input bitstream buffer so some overrun is
permssible without seg fault and only check position at the end of every
block or other medium sized unit. (You can nearly always work out what
the worst case overread can be.)

>2: your indentation of function declarations is weird. E.g.:
>
>+static inline uint32_t get_greaterx_bits(HEVCContext * const s, const
>unsigned int n_end, int * const levels,
>+    int * const pprev_subset_coded, int * const psum,
>+    const unsigned int idx0_gt1, const unsigned int idx_gt2)
>
>We tend to indent the second line so it aligns with the opening bracket of
>the first line.

Fair enough

>Alike, your indentation of const variable declarations:
>
>+    uint8_t * const state0 = s->HEVClc->cabac_state + idx0_gt1;
>
>doesn't need a space between * and const.

If that is required style then I'll abide by it, but I think that
detracts noticably from readability.

>Like I said, all non-technical, I'll do technical bits soon if I find time.
>If other people like it and I haven't responded yet, just commit it and we
>can address them post-push.

Thanks

JC

>Ronald
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel at ffmpeg.org
>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel