| Message ID | 20180510014859.26023-1-philipl@overt.org |
|---|---|
| State | Accepted |
| Commit | 1261003700322789d62a892e3325f8b58349d051 |
| Headers | show |
Am 10.05.2018 um 03:48 schrieb Philip Langdale: > The main issue here was the use of [i] instead of [i * 3] for the 32x32 > matrix. As part of fixing this, I changed the code to match that used > in vdpau_hevc, which I spent a lot of time verifying. > > I also changed to calculating NumPocTotalCurr using the existing helper, > which is what vdpau does. > --- > libavcodec/nvdec_hevc.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/nvdec_hevc.c b/libavcodec/nvdec_hevc.c > index 008963130b..e04a701f3a 100644 > --- a/libavcodec/nvdec_hevc.c > +++ b/libavcodec/nvdec_hevc.c > @@ -58,12 +58,13 @@ static void fill_scaling_lists(CUVIDHEVCPICPARAMS *ppc, const HEVCContext *s) > ppc->ScalingList16x16[i][j] = sl->sl[2][i][pos]; > > if (i < 2) > - ppc->ScalingList32x32[i][j] = sl->sl[3][i][pos]; > + ppc->ScalingList32x32[i][j] = sl->sl[3][i * 3][pos]; > } > - } > > - memcpy(ppc->ScalingListDCCoeff16x16, sl->sl_dc[0], sizeof(ppc->ScalingListDCCoeff16x16)); > - memcpy(ppc->ScalingListDCCoeff32x32, sl->sl_dc[1], sizeof(ppc->ScalingListDCCoeff32x32)); > + ppc->ScalingListDCCoeff16x16[i] = sl->sl_dc[0][i]; > + if (i < 2) > + ppc->ScalingListDCCoeff32x32[i] = sl->sl_dc[1][i * 3]; > + } > } > > static int nvdec_hevc_start_frame(AVCodecContext *avctx, > @@ -166,8 +167,7 @@ static int nvdec_hevc_start_frame(AVCodecContext *avctx, > > .NumBitsForShortTermRPSInSlice = s->sh.short_term_rps ? s->sh.short_term_ref_pic_set_size : 0, > .NumDeltaPocsOfRefRpsIdx = s->sh.short_term_rps ? s->sh.short_term_rps->rps_idx_num_delta_pocs : 0, > - .NumPocTotalCurr = s->rps[ST_CURR_BEF].nb_refs + s->rps[ST_CURR_AFT].nb_refs + > - s->rps[LT_CURR].nb_refs, > + .NumPocTotalCurr = ff_hevc_frame_nb_refs(s), This produces a warning because ff_hevc_frame_nb_refs discards the const from s. I don't really see a reason why it would need to do that, so I'd say the apropiate fix is to make ff_hevc_frame_nb_refs take const instead.
On Thu, 10 May 2018 12:18:13 +0200 Timo Rothenpieler <timo@rothenpieler.org> wrote: > Am 10.05.2018 um 03:48 schrieb Philip Langdale: > > The main issue here was the use of [i] instead of [i * 3] for the > > 32x32 matrix. As part of fixing this, I changed the code to match > > that used in vdpau_hevc, which I spent a lot of time verifying. > > > > I also changed to calculating NumPocTotalCurr using the existing > > helper, which is what vdpau does. > > --- > > libavcodec/nvdec_hevc.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/nvdec_hevc.c b/libavcodec/nvdec_hevc.c > > index 008963130b..e04a701f3a 100644 > > --- a/libavcodec/nvdec_hevc.c > > +++ b/libavcodec/nvdec_hevc.c > > @@ -58,12 +58,13 @@ static void > > fill_scaling_lists(CUVIDHEVCPICPARAMS *ppc, const HEVCContext *s) > > ppc->ScalingList16x16[i][j] = sl->sl[2][i][pos]; > > if (i < 2) > > - ppc->ScalingList32x32[i][j] = sl->sl[3][i][pos]; > > + ppc->ScalingList32x32[i][j] = sl->sl[3][i * > > 3][pos]; } > > - } > > > > - memcpy(ppc->ScalingListDCCoeff16x16, sl->sl_dc[0], > > sizeof(ppc->ScalingListDCCoeff16x16)); > > - memcpy(ppc->ScalingListDCCoeff32x32, sl->sl_dc[1], > > sizeof(ppc->ScalingListDCCoeff32x32)); > > + ppc->ScalingListDCCoeff16x16[i] = sl->sl_dc[0][i]; > > + if (i < 2) > > + ppc->ScalingListDCCoeff32x32[i] = sl->sl_dc[1][i * 3]; > > + } > > } > > > > static int nvdec_hevc_start_frame(AVCodecContext *avctx, > > @@ -166,8 +167,7 @@ static int > > nvdec_hevc_start_frame(AVCodecContext *avctx, > > .NumBitsForShortTermRPSInSlice = > > s->sh.short_term_rps ? s->sh.short_term_ref_pic_set_size : > > 0, .NumDeltaPocsOfRefRpsIdx = > > s->sh.short_term_rps ? > > s->sh.short_term_rps->rps_idx_num_delta_pocs : 0, > > - .NumPocTotalCurr = > > s->rps[ST_CURR_BEF].nb_refs + s->rps[ST_CURR_AFT].nb_refs + > > - > > s->rps[LT_CURR].nb_refs, > > + .NumPocTotalCurr = > > ff_hevc_frame_nb_refs(s), > > This produces a warning because ff_hevc_frame_nb_refs discards the > const from s. > I don't really see a reason why it would need to do that, so I'd say > the apropiate fix is to make ff_hevc_frame_nb_refs take const instead. > Yes - I noticed this but then forgot to address it. I'll push after you push your change to make it const. Thanks. --phil
applied and backported to 4.0
diff --git a/libavcodec/nvdec_hevc.c b/libavcodec/nvdec_hevc.c index 008963130b..e04a701f3a 100644 --- a/libavcodec/nvdec_hevc.c +++ b/libavcodec/nvdec_hevc.c @@ -58,12 +58,13 @@ static void fill_scaling_lists(CUVIDHEVCPICPARAMS *ppc, const HEVCContext *s) ppc->ScalingList16x16[i][j] = sl->sl[2][i][pos]; if (i < 2) - ppc->ScalingList32x32[i][j] = sl->sl[3][i][pos]; + ppc->ScalingList32x32[i][j] = sl->sl[3][i * 3][pos]; } - } - memcpy(ppc->ScalingListDCCoeff16x16, sl->sl_dc[0], sizeof(ppc->ScalingListDCCoeff16x16)); - memcpy(ppc->ScalingListDCCoeff32x32, sl->sl_dc[1], sizeof(ppc->ScalingListDCCoeff32x32)); + ppc->ScalingListDCCoeff16x16[i] = sl->sl_dc[0][i]; + if (i < 2) + ppc->ScalingListDCCoeff32x32[i] = sl->sl_dc[1][i * 3]; + } } static int nvdec_hevc_start_frame(AVCodecContext *avctx, @@ -166,8 +167,7 @@ static int nvdec_hevc_start_frame(AVCodecContext *avctx, .NumBitsForShortTermRPSInSlice = s->sh.short_term_rps ? s->sh.short_term_ref_pic_set_size : 0, .NumDeltaPocsOfRefRpsIdx = s->sh.short_term_rps ? s->sh.short_term_rps->rps_idx_num_delta_pocs : 0, - .NumPocTotalCurr = s->rps[ST_CURR_BEF].nb_refs + s->rps[ST_CURR_AFT].nb_refs + - s->rps[LT_CURR].nb_refs, + .NumPocTotalCurr = ff_hevc_frame_nb_refs(s), .NumPocStCurrBefore = s->rps[ST_CURR_BEF].nb_refs, .NumPocStCurrAfter = s->rps[ST_CURR_AFT].nb_refs, .NumPocLtCurr = s->rps[LT_CURR].nb_refs,