[FFmpeg-devel] [PATCH] lavc/vaapi_encode_h264: fix poc incorrect issue after meeting idr frame.

[Mark Thompson]

On 11/11/16 07:02, Jun Zhao wrote:
> From 25a5cc94fba53405acd53f9613fa5d206ce748f0 Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.zhao at intel.com>
> Date: Fri, 11 Nov 2016 14:53:49 +0800
> Subject: [PATCH] lavc/vaapi_encode_h264: fix poc incorrect issue after meeting
>  idr frame.
>
> when meeting IDR frame, vaapi_encode_h264 poc sometime is wrong, now fix
> this issue based on h264 spec.
>
> Reviewed-by: Jun Zhao <jun.zhao at intel.com>
> Signed-off-by: Wang, Yi A <yi.a.wang at intel.com>
>
> ase enter the commit message for your changes. Lines starting
> ---
>  libavcodec/vaapi_encode_h264.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

This is correct.  Can you modify the commit message to actually explain the problem more fully?  It took me a while to work out exactly what you were getting at, especially because the reference decoder does not care about it.


> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 5bed4e4..9e526c8 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -148,6 +148,8 @@ typedef struct VAAPIEncodeH264Context {
>
>      // Rate control configuration.
>      int send_timing_sei;
> +
> +    int64_t last_idr_frame;

It's not a rate control parameter.  Move it to immediately after next_frame_num, maybe?


>      struct {
>          VAEncMiscParameterBuffer misc;
>          VAEncMiscParameterRateControl rc;
> @@ -947,6 +949,7 @@ static int vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>          vpic->frame_num = 0;
>          priv->next_frame_num = 1;
>          priv->cpb_delay = 0;
> +        priv->last_idr_frame = pic->display_order;
>      } else {
>          vpic->frame_num = priv->next_frame_num;
>          if (pic->type != PICTURE_TYPE_B) {
> @@ -963,8 +966,8 @@ static int vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>      vpic->CurrPic.picture_id          = pic->recon_surface;
>      vpic->CurrPic.frame_idx           = vpic->frame_num;
>      vpic->CurrPic.flags               = 0;
> -    vpic->CurrPic.TopFieldOrderCnt    = pic->display_order;
> -    vpic->CurrPic.BottomFieldOrderCnt = pic->display_order;
> +    vpic->CurrPic.TopFieldOrderCnt    = pic->display_order - priv->last_idr_frame;
> +    vpic->CurrPic.BottomFieldOrderCnt = pic->display_order - priv->last_idr_frame;
>
>      for (i = 0; i < pic->nb_refs; i++) {
>          VAAPIEncodePicture *ref = pic->refs[i];
> @@ -972,8 +975,8 @@ static int vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>          vpic->ReferenceFrames[i].picture_id = ref->recon_surface;
>          vpic->ReferenceFrames[i].frame_idx  = ref->encode_order;
>          vpic->ReferenceFrames[i].flags = VA_PICTURE_H264_SHORT_TERM_REFERENCE;
> -        vpic->ReferenceFrames[i].TopFieldOrderCnt    = ref->display_order;
> -        vpic->ReferenceFrames[i].BottomFieldOrderCnt = ref->display_order;
> +        vpic->ReferenceFrames[i].TopFieldOrderCnt    = ref->display_order - priv->last_idr_frame;
> +        vpic->ReferenceFrames[i].BottomFieldOrderCnt = ref->display_order - priv->last_idr_frame;
>      }
>      for (; i < FF_ARRAY_ELEMS(vpic->ReferenceFrames); i++) {
>          vpic->ReferenceFrames[i].picture_id = VA_INVALID_ID;
> @@ -1044,7 +1047,7 @@ static int vaapi_encode_h264_init_slice_params(AVCodecContext *avctx,
>      vslice->pic_parameter_set_id = vpic->pic_parameter_set_id;
>      vslice->idr_pic_id = priv->idr_pic_count++;
>
> -    vslice->pic_order_cnt_lsb = pic->display_order &
> +    vslice->pic_order_cnt_lsb = (pic->display_order - priv->last_idr_frame) &
>          ((1 << (4 + vseq->seq_fields.bits.log2_max_pic_order_cnt_lsb_minus4)) - 1);
>
>      for (i = 0; i < FF_ARRAY_ELEMS(vslice->RefPicList0); i++) {
> --
> 1.8.3.1
>

Actual code LGTM.


Thanks,

- Mark