GNOME Bugzilla – Bug 783804
encoder: h265(HEVC): Add multi reference support
Last modified: 2017-09-13 09:14:22 UTC
Currently we only support 1 ref frame in P frame encode and 2 ref frames in B frame encode. With the recent changes in Intel-vaapi-driver(SKL, BXT and KBL platforms), we should be able to use multi-referencing. -- Query the driver with VAConfigAttribEncMaxRefFrames and find the number of reference frames supported. Need a fix in driver:Currently the value is hard-coded in driver query implementation. -- Implement a basic algorithm in vaapih265encoder to utilize the multi referencing. For eg: use the previous frames(n-1, n-2...n-x, where n=current frame, x=user-input-number-of-ref-frames-to-be-used) for referencing.
Created attachment 356824 [details] [review] libs: encoder: h265: add refs property Users can provide the number of reference frame by this property, which is exaclty same as h264. The value of the property will be considered as the number of reference picture list0 and will add 1 reference frame more to the reference picture list1 internally if b-frame encoding. If the value provided is bigger than the number of refrence frames supported in the driver, it will be lowered.
Created attachment 356825 [details] [review] libs: encoder: h265: add multi reference support This is doing the same as h264 encoder as the following: Using num_ref_frames provided and the result of the Query VAConfigAttribEncMaxRefFrames, it determines the size of reference list and perform encoding with multi reference frames as the following: 1\ The num_ref_frames is being considered as the number of reference picture list0 2\ Encoder adds 1 reference frame more to the reference picture list1 internally if b-frame encoding. 3\ If num_ref_frames is bigger than the number of refrence frames supported in the driver, it will be lowered.
Currently, the driver support 1 ref frame for each list on HEVC, which means that these patches are doing the same thing as before. Maybe we might have to be waiting for status in the driver.
(In reply to Hyunjun Ko from comment #3) > Currently, the driver support 1 ref frame for each list on HEVC, > which means that these patches are doing the same thing as before. > > Maybe we might have to be waiting for status in the driver. I tested on SKL/KBL, btw.
This patch in the driver enables multi-ref HEVC encoding. https://github.com/01org/intel-vaapi-driver/pull/244 And I found some problems current proposed patch. I'm working on it based on the patch.
Created attachment 357376 [details] [review] libs: encoder: h265: add refs property Users can provide the number of reference frame by this property, which is exaclty same as h264. The value of the property will be considered as the number of reference picture list0 and will add 1 reference frame more to the reference picture list1 internally if b-frame encoding. If the value provided is bigger than the number of refrence frames supported in the driver, it will be lowered. The maximum value is aligned to the value of the driver supported now.
Created attachment 357377 [details] [review] libs: encoder: h265: add multi reference support This is doing the same as h264 encoder as the following: Using num_ref_frames provided and the result of the Query VAConfigAttribEncMaxRefFrames, it determines the size of reference list and perform encoding with multi reference frames as the following: 1\ The num_ref_frames is being considered as the number of reference picture list0 2\ Encoder adds 1 reference frame more to the reference picture list1 internally if b-frame encoding. 3\ If num_ref_frames is bigger than the number of refrence frames supported in the driver, it will be lowered. Also this patch includes: - Set num_negative_pics and num_positive_pics according to the number of refs. - Set delta_poc according to the number of refs. - Set num_ref_idx_active_override_flag to TRUE and add num_ref_idx_l0_active_minus1 and num_ref_idx_l1_active_minus1 to slice. - Increase max_dec_pic_buffering according to the number of refs - Change max_num_reorder_pics according to num of bframes - Change to keep IDR period and keyframe-period equal.
Review of attachment 357376 [details] [review]: lgtm
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. @@ +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. @@ +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
(In reply to Víctor Manuel Jáquez Leal from comment #9) > Review of attachment 357377 [details] [review] [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!
(In reply to Hyunjun Ko from comment #10) > > > > @@ +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! Unfortunately, num_bframes is private for h264/5, which means gst_vaapi_encoder_ensure_max_num_ref_frames can't access the variable.
Created attachment 359677 [details] [review] libs: encoder: h265: keep idr_period equal to keyframe period Remove FIXME code, which makes previous assignation spurious. This also means to make idr_period equal to keyframe period, which is same as h264 encoder.
Created attachment 359678 [details] [review] libs: encoder: h264/5: determine num_ref_idx_active_override_flag according to reference list Follows the specification as below: 7.4.7.1 in Rec. ITU-T H.265 v4 (12/2016) 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. https://bugzilla.gnome.org/show_bug.cgi?id=783804
Created attachment 359679 [details] [review] libs: encoder: h265: add refs property Users can provide the number of reference frame by this property, which is exaclty same as h264. The value of the property will be considered as the number of reference picture list0 and will add 1 reference frame more to the reference picture list1 internally if b-frame encoding. If the value provided is bigger than the number of refrence frames supported in the driver, it will be lowered. The maximum value is aligned to the value of the driver supported now.
Created attachment 359680 [details] [review] libs: encoder: h265: add multi reference support This is doing the same as h264 encoder as the following: Using num_ref_frames provided and the result of the Query VAConfigAttribEncMaxRefFrames, it determines the size of reference list and perform encoding with multi reference frames as the following: 1\ The num_ref_frames is being considered as the number of reference picture list0 2\ Encoder adds 1 reference frame more to the reference picture list1 internally if b-frame encoding. 3\ If num_ref_frames is bigger than the number of refrence frames supported in the driver, it will be lowered. Also this patch includes: - Set num_negative_pics and num_positive_pics according to the number of refs. - Set delta_poc according to the number of refs. - Increase max_dec_pic_buffering according to the number of refs - Change max_num_reorder_pics according to num of bframes
Attachment 359679 [details] pushed as 3dbf440 - libs: encoder: h265: add refs property Attachment 359680 [details] pushed as aa6aa99 - libs: encoder: h265: add multi reference support Attachment 359677 [details] pushed as f788600 - libs: encoder: h265: keep idr_period equal to keyframe period Attachment 359678 [details] pushed as f145637 - libs: encoder: h264/5: determine num_ref_idx_active_override_flag according to reference list