diff mbox

[FFmpeg-devel] lavc/hevc_cabac: fix cbf_cb and cbf_cr for transform depth 4

Message ID 1576054058-1518-1-git-send-email-linjie.fu@intel.com
State Accepted
Commit d31a2902261072f8195a005b78b4e0c4a973bf80
Headers show

Commit Message

Fu, Linjie Dec. 11, 2019, 8:47 a.m. UTC 
The max transform depth is 5(from 0 to 4), so we need 5 cabac states for
cbf_cb and cbf_cr.

See Table 9-4 for details.

Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>
Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/hevc_cabac.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

Comments

Carl Eugen Hoyos Dec. 11, 2019, 10:18 a.m. UTC | #1 
Am Mi., 11. Dez. 2019 um 09:57 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:
>
> The max transform depth is 5(from 0 to 4), so we need 5 cabac states for
> cbf_cb and cbf_cr.
>
> See Table 9-4 for details.

Does this change the output of a file?
If yes, please provide it / implement a fate test.

Carl Eugen
Fu, Linjie Dec. 12, 2019, 7:27 a.m. UTC | #2 
> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> Carl Eugen Hoyos

> Sent: Wednesday, December 11, 2019 18:19

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and cbf_cr

> for transform depth 4

> 

> Am Mi., 11. Dez. 2019 um 09:57 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:

> >

> > The max transform depth is 5(from 0 to 4), so we need 5 cabac states for

> > cbf_cb and cbf_cr.

> >

> > See Table 9-4 for details.

> 

> Does this change the output of a file?

> If yes, please provide it / implement a fate test.


Yes, this issue was found in some license-limited clips and does affect the decoding. 
Hence it takes some efforts to generate an available file for verifying/test in the
community.

I've created a ticket for this and attached the clips and the decode result with/without
this patch:
https://trac.ffmpeg.org/ticket/8425

Please help to verify.

Thx,

- linjie
Thilo Borgmann Dec. 12, 2019, 2:14 p.m. UTC | #3 
Hi,

Am 12.12.19 um 08:27 schrieb Fu, Linjie:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Carl Eugen Hoyos
>> Sent: Wednesday, December 11, 2019 18:19
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and cbf_cr
>> for transform depth 4
>>
>> Am Mi., 11. Dez. 2019 um 09:57 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:
>>>
>>> The max transform depth is 5(from 0 to 4), so we need 5 cabac states for
>>> cbf_cb and cbf_cr.
>>>
>>> See Table 9-4 for details.
>>
>> Does this change the output of a file?
>> If yes, please provide it / implement a fate test.
> 
> Yes, this issue was found in some license-limited clips and does affect the decoding. 
> Hence it takes some efforts to generate an available file for verifying/test in the
> community.
> 
> I've created a ticket for this and attached the clips and the decode result with/without
> this patch:
> https://trac.ffmpeg.org/ticket/8425
> 
> Please help to verify.

please add a fate test that uses your .h265 sample to verify decoding.

Have a look at previous patches adding fate tests if you are uncertain what to do.

I'll upload the sample once it is used in fate.

-Thilo
Fu, Linjie Dec. 12, 2019, 3:43 p.m. UTC | #4 
> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> Thilo Borgmann

> Sent: Thursday, December 12, 2019 22:15

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and cbf_cr

> for transform depth 4

> 

> Hi,

> 

> Am 12.12.19 um 08:27 schrieb Fu, Linjie:

> >> -----Original Message-----

> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> >> Carl Eugen Hoyos

> >> Sent: Wednesday, December 11, 2019 18:19

> >> To: FFmpeg development discussions and patches <ffmpeg-

> >> devel@ffmpeg.org>

> >> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and

> cbf_cr

> >> for transform depth 4

> >>

> >> Am Mi., 11. Dez. 2019 um 09:57 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:

> >>>

> >>> The max transform depth is 5(from 0 to 4), so we need 5 cabac states for

> >>> cbf_cb and cbf_cr.

> >>>

> >>> See Table 9-4 for details.

> >>

> >> Does this change the output of a file?

> >> If yes, please provide it / implement a fate test.

> >

> > Yes, this issue was found in some license-limited clips and does affect the

> decoding.

> > Hence it takes some efforts to generate an available file for verifying/test

> in the

> > community.

> >

> > I've created a ticket for this and attached the clips and the decode result

> with/without

> > this patch:

> > https://trac.ffmpeg.org/ticket/8425

> >

> > Please help to verify.

> 

> please add a fate test that uses your .h265 sample to verify decoding.

> 

> Have a look at previous patches adding fate tests if you are uncertain what to

> do.

> 

> I'll upload the sample once it is used in fate.

> 


Renamed the file to hevc-cabac-tudepth.hevc and sent the fate test patch.
Thanks for the kindness.

- linjie
Fu, Linjie Dec. 15, 2019, 1:56 a.m. UTC | #5 
> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> Thilo Borgmann

> Sent: Thursday, December 12, 2019 22:15

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and cbf_cr

> for transform depth 4

> 

> Hi,

> 

> Am 12.12.19 um 08:27 schrieb Fu, Linjie:

> >> -----Original Message-----

> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> >> Carl Eugen Hoyos

> >> Sent: Wednesday, December 11, 2019 18:19

> >> To: FFmpeg development discussions and patches <ffmpeg-

> >> devel@ffmpeg.org>

> >> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and

> cbf_cr

> >> for transform depth 4

> >>

> >> Am Mi., 11. Dez. 2019 um 09:57 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:

> >>>

> >>> The max transform depth is 5(from 0 to 4), so we need 5 cabac states for

> >>> cbf_cb and cbf_cr.

> >>>

> >>> See Table 9-4 for details.

> >>

> >> Does this change the output of a file?

> >> If yes, please provide it / implement a fate test.

> >

> > Yes, this issue was found in some license-limited clips and does affect the

> decoding.

> > Hence it takes some efforts to generate an available file for verifying/test

> in the

> > community.

> >

> > I've created a ticket for this and attached the clips and the decode result

> with/without

> > this patch:

> > https://trac.ffmpeg.org/ticket/8425

> >

> > Please help to verify.

> 

> please add a fate test that uses your .h265 sample to verify decoding.

> 

Ping for this patch and the test in fate, thx.

- linjie
Fu, Linjie Dec. 17, 2019, 4:17 p.m. UTC | #6 
Hi,

> -----Original Message-----

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu,

> Linjie

> Sent: Sunday, December 15, 2019 09:56

> To: FFmpeg development discussions and patches <ffmpeg-

> devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and cbf_cr

> for transform depth 4

> 

> > -----Original Message-----

> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of

> > Thilo Borgmann

> > Sent: Thursday, December 12, 2019 22:15

> > To: ffmpeg-devel@ffmpeg.org

> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and

> cbf_cr

> > for transform depth 4

> >

> > Hi,

> >

> > Am 12.12.19 um 08:27 schrieb Fu, Linjie:

> > >> -----Original Message-----

> > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf

> Of

> > >> Carl Eugen Hoyos

> > >> Sent: Wednesday, December 11, 2019 18:19

> > >> To: FFmpeg development discussions and patches <ffmpeg-

> > >> devel@ffmpeg.org>

> > >> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_cabac: fix cbf_cb and

> > cbf_cr

> > >> for transform depth 4

> > >>

> > >> Am Mi., 11. Dez. 2019 um 09:57 Uhr schrieb Linjie Fu

> <linjie.fu@intel.com>:

> > >>>

> > >>> The max transform depth is 5(from 0 to 4), so we need 5 cabac states

> for

> > >>> cbf_cb and cbf_cr.

> > >>>

> > >>> See Table 9-4 for details.

> > >>

> > >> Does this change the output of a file?

> > >> If yes, please provide it / implement a fate test.

> > >

> > > Yes, this issue was found in some license-limited clips and does affect the

> > decoding.

> > > Hence it takes some efforts to generate an available file for verifying/test

> > in the

> > > community.

> > >

> > > I've created a ticket for this and attached the clips and the decode result

> > with/without

> > > this patch:

> > > https://trac.ffmpeg.org/ticket/8425

> > >

> > > Please help to verify.

> >

> > please add a fate test that uses your .h265 sample to verify decoding.

> >

> Ping for this patch and the test in fate, thx.

> 

Ping in case it is missed, thx.

- linjie
James Almer Dec. 17, 2019, 5:22 p.m. UTC | #7 
On 12/11/2019 5:47 AM, Linjie Fu wrote:
> The max transform depth is 5(from 0 to 4), so we need 5 cabac states for
> cbf_cb and cbf_cr.
> 
> See Table 9-4 for details.
> 
> Signed-off-by: Xu Guangxin <guangxin.xu@intel.com>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavcodec/hevc_cabac.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/libavcodec/hevc_cabac.c b/libavcodec/hevc_cabac.c
> index 8abb780..3dc0987 100644
> --- a/libavcodec/hevc_cabac.c
> +++ b/libavcodec/hevc_cabac.c
> @@ -66,7 +66,7 @@ static const int8_t num_bins_in_se[] = {
>       1, // no_residual_data_flag
>       3, // split_transform_flag
>       2, // cbf_luma
> -     4, // cbf_cb, cbf_cr
> +     5, // cbf_cb, cbf_cr
>       2, // transform_skip_flag[][]
>       2, // explicit_rdpcm_flag[][]
>       2, // explicit_rdpcm_dir_flag[][]
> @@ -122,23 +122,23 @@ static const int elem_offset[sizeof(num_bins_in_se)] = {
>      37, // split_transform_flag
>      40, // cbf_luma
>      42, // cbf_cb, cbf_cr
> -    46, // transform_skip_flag[][]
> -    48, // explicit_rdpcm_flag[][]
> -    50, // explicit_rdpcm_dir_flag[][]
> -    52, // last_significant_coeff_x_prefix
> -    70, // last_significant_coeff_y_prefix
> -    88, // last_significant_coeff_x_suffix
> -    88, // last_significant_coeff_y_suffix
> -    88, // significant_coeff_group_flag
> -    92, // significant_coeff_flag
> -    136, // coeff_abs_level_greater1_flag
> -    160, // coeff_abs_level_greater2_flag
> -    166, // coeff_abs_level_remaining
> -    166, // coeff_sign_flag
> -    166, // log2_res_scale_abs
> -    174, // res_scale_sign_flag
> -    176, // cu_chroma_qp_offset_flag
> -    177, // cu_chroma_qp_offset_idx
> +    47, // transform_skip_flag[][]
> +    49, // explicit_rdpcm_flag[][]
> +    51, // explicit_rdpcm_dir_flag[][]
> +    53, // last_significant_coeff_x_prefix
> +    71, // last_significant_coeff_y_prefix
> +    89, // last_significant_coeff_x_suffix
> +    89, // last_significant_coeff_y_suffix
> +    89, // significant_coeff_group_flag
> +    93, // significant_coeff_flag
> +    137, // coeff_abs_level_greater1_flag
> +    161, // coeff_abs_level_greater2_flag
> +    167, // coeff_abs_level_remaining
> +    167, // coeff_sign_flag
> +    167, // log2_res_scale_abs
> +    175, // res_scale_sign_flag
> +    177, // cu_chroma_qp_offset_flag
> +    178, // cu_chroma_qp_offset_idx
>  };
>  
>  #define CNU 154
> @@ -189,7 +189,7 @@ static const uint8_t init_values[3][HEVC_CONTEXTS] = {
>        // cbf_luma
>        111, 141,
>        // cbf_cb, cbf_cr
> -      94, 138, 182, 154,
> +      94, 138, 182, 154, 154,
>        // transform_skip_flag
>        139, 139,
>        // explicit_rdpcm_flag
> @@ -266,7 +266,7 @@ static const uint8_t init_values[3][HEVC_CONTEXTS] = {
>        // cbf_luma
>        153, 111,
>        // cbf_cb, cbf_cr
> -      149, 107, 167, 154,
> +      149, 107, 167, 154, 154,
>        // transform_skip_flag
>        139, 139,
>        // explicit_rdpcm_flag
> @@ -343,7 +343,7 @@ static const uint8_t init_values[3][HEVC_CONTEXTS] = {
>        // cbf_luma
>        153, 111,
>        // cbf_cb, cbf_cr
> -      149, 92, 167, 154,
> +      149, 92, 167, 154, 154,
>        // transform_skip_flag
>        139, 139,
>        // explicit_rdpcm_flag

Applied, thanks.
diff mbox

Patch

diff --git a/libavcodec/hevc_cabac.c b/libavcodec/hevc_cabac.c
index 8abb780..3dc0987 100644
--- a/libavcodec/hevc_cabac.c
+++ b/libavcodec/hevc_cabac.c
@@ -66,7 +66,7 @@  static const int8_t num_bins_in_se[] = {
      1, // no_residual_data_flag
      3, // split_transform_flag
      2, // cbf_luma
-     4, // cbf_cb, cbf_cr
+     5, // cbf_cb, cbf_cr
      2, // transform_skip_flag[][]
      2, // explicit_rdpcm_flag[][]
      2, // explicit_rdpcm_dir_flag[][]
@@ -122,23 +122,23 @@  static const int elem_offset[sizeof(num_bins_in_se)] = {
     37, // split_transform_flag
     40, // cbf_luma
     42, // cbf_cb, cbf_cr
-    46, // transform_skip_flag[][]
-    48, // explicit_rdpcm_flag[][]
-    50, // explicit_rdpcm_dir_flag[][]
-    52, // last_significant_coeff_x_prefix
-    70, // last_significant_coeff_y_prefix
-    88, // last_significant_coeff_x_suffix
-    88, // last_significant_coeff_y_suffix
-    88, // significant_coeff_group_flag
-    92, // significant_coeff_flag
-    136, // coeff_abs_level_greater1_flag
-    160, // coeff_abs_level_greater2_flag
-    166, // coeff_abs_level_remaining
-    166, // coeff_sign_flag
-    166, // log2_res_scale_abs
-    174, // res_scale_sign_flag
-    176, // cu_chroma_qp_offset_flag
-    177, // cu_chroma_qp_offset_idx
+    47, // transform_skip_flag[][]
+    49, // explicit_rdpcm_flag[][]
+    51, // explicit_rdpcm_dir_flag[][]
+    53, // last_significant_coeff_x_prefix
+    71, // last_significant_coeff_y_prefix
+    89, // last_significant_coeff_x_suffix
+    89, // last_significant_coeff_y_suffix
+    89, // significant_coeff_group_flag
+    93, // significant_coeff_flag
+    137, // coeff_abs_level_greater1_flag
+    161, // coeff_abs_level_greater2_flag
+    167, // coeff_abs_level_remaining
+    167, // coeff_sign_flag
+    167, // log2_res_scale_abs
+    175, // res_scale_sign_flag
+    177, // cu_chroma_qp_offset_flag
+    178, // cu_chroma_qp_offset_idx
 };
 
 #define CNU 154
@@ -189,7 +189,7 @@  static const uint8_t init_values[3][HEVC_CONTEXTS] = {
       // cbf_luma
       111, 141,
       // cbf_cb, cbf_cr
-      94, 138, 182, 154,
+      94, 138, 182, 154, 154,
       // transform_skip_flag
       139, 139,
       // explicit_rdpcm_flag
@@ -266,7 +266,7 @@  static const uint8_t init_values[3][HEVC_CONTEXTS] = {
       // cbf_luma
       153, 111,
       // cbf_cb, cbf_cr
-      149, 107, 167, 154,
+      149, 107, 167, 154, 154,
       // transform_skip_flag
       139, 139,
       // explicit_rdpcm_flag
@@ -343,7 +343,7 @@  static const uint8_t init_values[3][HEVC_CONTEXTS] = {
       // cbf_luma
       153, 111,
       // cbf_cb, cbf_cr
-      149, 92, 167, 154,
+      149, 92, 167, 154, 154,
       // transform_skip_flag
       139, 139,
       // explicit_rdpcm_flag