diff mbox series

[FFmpeg-devel] avcodec/hevc_cabac: cabac_init_state, do not use magic number which not listed on the spec

Message ID 20210321050611.32380-1-nuomi2021@gmail.com
State New
Headers show
Series [FFmpeg-devel] avcodec/hevc_cabac: cabac_init_state, do not use magic number which not listed on the spec | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Nuo Mi March 21, 2021, 5:06 a.m. UTC 
Magic number 124 and ^= are not listed on the spec.
Strictly following the spec will make a reader's life much easier. See (9-6) for details
---
 libavcodec/hevc_cabac.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Carl Eugen Hoyos March 21, 2021, 8:33 a.m. UTC | #1 
Am So., 21. März 2021 um 06:13 Uhr schrieb Nuo Mi <nuomi2021@gmail.com>:
>
> Magic number 124 and ^= are not listed on the spec.
> Strictly following the spec will make a reader's life much easier. See (9-6) for details

But FFmpeg is not providing a reference application for the reader,
this is software actually used by real people (both as a library in
other projects and in the ffmpeg application).

Afaict, the question is if the hevc cabac reader is faster or slower
with your patch.
It looks to me as if you are replacing one condition with two conditions.

Carl Eugen
Nuo Mi March 21, 2021, 1:03 p.m. UTC | #2 
On Sun, Mar 21, 2021 at 4:33 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> Am So., 21. März 2021 um 06:13 Uhr schrieb Nuo Mi <nuomi2021@gmail.com>:
> >
> > Magic number 124 and ^= are not listed on the spec.
> > Strictly following the spec will make a reader's life much easier. See
> (9-6) for details
>
> But FFmpeg is not providing a reference application for the reader,
> this is software actually used by real people (both as a library in
> other projects and in the ffmpeg application).
>
> Afaict, the question is if the hevc cabac reader is faster or slower
> with your patch.
> It looks to me as if you are replacing one condition with two conditions.
>
Hi Carl,
Thanks for the review.
It's not a time-sensitive code. We only call them at tile/slice start.
A readable code will help us fix bug easier.

>
> Carl Eugen
diff mbox series

Patch

diff --git a/libavcodec/hevc_cabac.c b/libavcodec/hevc_cabac.c
index 9b8c8e342d..7ac340f471 100644
--- a/libavcodec/hevc_cabac.c
+++ b/libavcodec/hevc_cabac.c
@@ -496,12 +496,10 @@  static void cabac_init_state(HEVCContext *s)
         int init_value = init_values[init_type][i];
         int m = (init_value >> 4) * 5 - 45;
         int n = ((init_value & 15) << 3) - 16;
-        int pre = 2 * (((m * av_clip(s->sh.slice_qp, 0, 51)) >> 4) + n) - 127;
-
-        pre ^= pre >> 31;
-        if (pre > 124)
-            pre = 124 + (pre & 1);
-        s->HEVClc->cabac_state[i] = pre;
+        int pre = av_clip(((m * av_clip(s->sh.slice_qp, 0, 51)) >> 4) + n, 1, 126);
+        int val_mps = (pre <= 63 ) ? 0 : 1;
+        int state = val_mps ? (pre - 64) : (63 - pre);
+        s->HEVClc->cabac_state[i] = (state << 1) + val_mps;
     }
 
     for (i = 0; i < 4; i++)