Message ID | 1576054058-1518-1-git-send-email-linjie.fu@intel.com |
---|---|
State | Accepted |
Commit | d31a2902261072f8195a005b78b4e0c4a973bf80 |
Headers | show |
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
> -----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
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
> -----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
> -----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
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
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 --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