[Bug 783804] encoder: h265(HEVC): Add multi reference support

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=783804

--- Comment #10 from Hyunjun Ko <zzoon at igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #9)
> Review of attachment 357377 [details] [review]:
> 
> ::: gst-libs/gst/vaapi/gstvaapiencoder_h265.c
> @@ +668,3 @@
>    guint8 dependent_slice_segment_flag = 0;
>    guint8 short_term_ref_pic_set_sps_flag = 0;
> +  guint8 num_ref_idx_active_override_flag = 1;
> 
> the spec says:
> 
> num_ref_idx_active_override_flag equal to 1 specifies that the syntax
> element num_ref_idx_l0_active_minus1 is present for P and B slices and that
> the syntax element num_ref_idx_l1_active_minus1 is present for B slices.
> num_ref_idx_active_override_flag equal to 0 specifies that the syntax
> elements num_ref_idx_l0_active_minus1 and num_ref_idx_l1_active_minus1 are
> not present.
> 
> so, if I understand correctly, we have to confirm it,
> num_ref_idx_active_override_flag is true only if
> num_ref_idx_l0_active_minus1 and num_ref_idx_l1_active_minus1 are bigger
> than zero. Isn't it?
> 
> Also, I think this should be in another patch.

Agree.

> 
> @@ +2009,2 @@
>    /* FIXME: provide user control for idr_period ?? */
> +  encoder->idr_period = base_encoder->keyframe_period;
> 
> this might be in another patch, since it is unrelated with refs. Also,
> explain why it is needed.

Okay.

> 
> @@ +2023,3 @@
> +      GST_VAAPI_ENTRYPOINT_SLICE_ENCODE);
> +
> +  if (base_encoder->max_num_ref_frames_1 < 1 && encoder->num_bframes > 0) {
> 
> To simplify this comparison, gst_vaapi_encoder_ensure_max_num_ref_frames()
> should return FALSE if this happens.
> 
> Also, happens in H264

I like this approach, I'm going to provide another patch for this. Thanks!

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.