From 81cf1c16db16326e8b6e540f20de60c46c940267 Mon Sep 17 00:00:00 2001 From: elwrcl Date: Fri, 21 Aug 2026 08:39:21 +0300 Subject: [PATCH] hasvk: Encode B pictures on gen7. The encoder refused B pictures outright. Three things were missing. The slice header writer marked every picture as a reference, nal_ref_idc hardcoded to 1. A B picture nothing will reference has to say so, otherwise a decoder keeps it in the DPB and its frame_num accounting drifts until the DPB overflows; the client's is_reference flag decides now, an IDR aside. The same writer emitted no B slice syntax at all, so direct_spatial_mv_pred_flag and the L1 reference list modification flag were absent and the bitstream went out of sync from that bit onwards. Spatial direct mode is chosen, which derives motion from neighbours rather than the co-located picture, so no direct MV buffer has to be read back. Finally the L1 reference list was never built. It comes from RefPicList1 the same way L0 comes from RefPicList0, and feeds MFX_AVC_REF_IDX_STATE list 1 and the L1 count in MFX_AVC_IMG_STATE. Prediction itself is unchanged: a B macroblock is coded as B_L0_16x16, so it predicts from L0 alone and costs about what a P macroblock costs. What a B picture buys here is that it need not enter the DPB, not a cheaper prediction. Bi-prediction would need the macroblock type raised to B_Bi_16x16 and motion vectors for both lists. dEQP-VK.video.encode.h264.i_p_b_13_* goes from NotSupported to 4/4 Pass. NOT FULLY VERIFIED: the run over the whole encode family aborted partway on an unrelated device loss, with two QualityWarnings in intra_refresh that were not there before, so this needs re-running on a healthy GPU before being trusted. Co-Authored-By: Claude Opus 5 --- src/intel/vulkan_hasvk/anv_private.h | 2 ++ src/intel/vulkan_hasvk/anv_video.c | 14 +++++----- .../vulkan_hasvk/anv_video_h264_bitstream.c | 23 ++++++++++++---- src/intel/vulkan_hasvk/anv_video_mfc.c | 26 +++++++++++-------- src/intel/vulkan_hasvk/genX_cmd_video.c | 8 ++++++ 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/intel/vulkan_hasvk/anv_private.h b/src/intel/vulkan_hasvk/anv_private.h index a61146f..9779696 100644 --- a/src/intel/vulkan_hasvk/anv_private.h +++ b/src/intel/vulkan_hasvk/anv_private.h @@ -3876,6 +3876,8 @@ struct anv_video_reference_state { bool slot_valid[ANV_VIDEO_MAX_DPB_SLOTS]; uint8_t l0_slot[ANV_VIDEO_MAX_DPB_SLOTS]; uint8_t l0_count; + uint8_t l1_slot[ANV_VIDEO_MAX_DPB_SLOTS]; + uint8_t l1_count; }; /* One picture's slice of an intra refresh cycle: the macroblocks of region diff --git a/src/intel/vulkan_hasvk/anv_video.c b/src/intel/vulkan_hasvk/anv_video.c index 2cf5a31..aa651c1 100644 --- a/src/intel/vulkan_hasvk/anv_video.c +++ b/src/intel/vulkan_hasvk/anv_video.c @@ -144,9 +144,8 @@ anv_GetPhysicalDeviceVideoCapabilitiesKHR(VkPhysicalDevice physicalDevice, } else { /* Baseline and Main are supported. Main is accepted so that CABAC * (entropy_coding_mode_flag == 1, forbidden in Baseline) can be selected; - * the PAK performs the entropy coding. Main's inter-prediction features - * (B slices) are still not emitted — only I frames are produced. High is - * rejected (8x8 transform / scaling lists not emitted). */ + * the PAK performs the entropy coding. High is rejected (8x8 transform / + * scaling lists not emitted). */ const VkVideoEncodeH264ProfileInfoKHR *h264_profile = vk_find_struct_const(pVideoProfile->pNext, VIDEO_ENCODE_H264_PROFILE_INFO_KHR); if (h264_profile) { @@ -208,7 +207,10 @@ anv_GetPhysicalDeviceVideoCapabilitiesKHR(VkPhysicalDevice physicalDevice, * of them can be predicted from, though: each macroblock is coded against * reference index 0, so multi-reference and B frames are still out. */ pCapabilities->maxDpbSlots = ANV_VIDEO_MAX_DPB_SLOTS; - pCapabilities->maxActiveReferencePictures = 1; + /* Two, so a B picture can name one reference in each list. Prediction + * still only uses reference index 0 of whichever list a macroblock is + * coded against. */ + pCapabilities->maxActiveReferencePictures = 2; struct VkVideoEncodeCapabilitiesKHR *enc_caps = (struct VkVideoEncodeCapabilitiesKHR *) @@ -274,8 +276,8 @@ anv_GetPhysicalDeviceVideoCapabilitiesKHR(VkPhysicalDevice physicalDevice, * slice splitting). */ h264_enc_ext->maxSliceCount = 68; /* 1088/16, covers up to 1080p */ h264_enc_ext->maxPPictureL0ReferenceCount = 1; - h264_enc_ext->maxBPictureL0ReferenceCount = 0; - h264_enc_ext->maxL1ReferenceCount = 0; + h264_enc_ext->maxBPictureL0ReferenceCount = 1; + h264_enc_ext->maxL1ReferenceCount = 1; h264_enc_ext->maxTemporalLayerCount = 1; h264_enc_ext->expectDyadicTemporalLayerPattern = VK_FALSE; h264_enc_ext->minQp = 0; diff --git a/src/intel/vulkan_hasvk/anv_video_h264_bitstream.c b/src/intel/vulkan_hasvk/anv_video_h264_bitstream.c index 02f2af2..c824730 100644 --- a/src/intel/vulkan_hasvk/anv_video_h264_bitstream.c +++ b/src/intel/vulkan_hasvk/anv_video_h264_bitstream.c @@ -108,7 +108,10 @@ anv_h264_build_encode_slice_header(uint8_t *out_buf, uint32_t out_cap, bool is_idr = pic_info->flags.IdrPicFlag; unsigned nal_unit_type = is_idr ? 5 : 1; - unsigned nal_ref_idc = 1; /* I-frame is always a reference */ + /* A picture nothing will reference must say so, or a decoder keeps it in + * the DPB and its frame_num accounting drifts: an IDR is always a + * reference, otherwise the client's is_reference decides. */ + unsigned nal_ref_idc = (is_idr || pic_info->flags.is_reference) ? 1 : 0; bw_put_bit(&bw, 0); /* forbidden_zero_bit */ bw_put_u(&bw, nal_ref_idc, 2); @@ -139,14 +142,24 @@ anv_h264_build_encode_slice_header(uint8_t *out_buf, uint32_t out_cap, if (pps->flags.redundant_pic_cnt_present_flag) bw_put_ue(&bw, 0); - /* P-slice reference-list syntax (H.264 7.3.3). B slices are out of scope, so - * direct_spatial_mv_pred_flag is never emitted. */ - bool is_p = (slice_hdr->slice_type == STD_VIDEO_H264_SLICE_TYPE_P); + /* Inter reference-list syntax (H.264 7.3.3). */ + bool is_b = (slice_hdr->slice_type == STD_VIDEO_H264_SLICE_TYPE_B); + bool is_p = is_b || (slice_hdr->slice_type == STD_VIDEO_H264_SLICE_TYPE_P); + if (is_b) { + /* direct_spatial_mv_pred_flag = 1: spatial direct mode, which derives + * its motion from neighbours instead of the co-located picture's stored + * vectors, so no direct MV buffer has to be read back. */ + bw_put_bit(&bw, 1); + } if (is_p) { - /* num_ref_idx_active_override_flag = 0: use the PPS default L0 count. */ + /* num_ref_idx_active_override_flag = 0: use the PPS default counts. */ bw_put_bit(&bw, 0); /* ref_pic_list_modification_flag_l0 = 0: default frame_num-ordered list. */ bw_put_bit(&bw, 0); + if (is_b) { + /* ref_pic_list_modification_flag_l1 = 0: default POC-ordered list. */ + bw_put_bit(&bw, 0); + } } /* pred_weight_table is absent (weighted_pred_flag is off). */ diff --git a/src/intel/vulkan_hasvk/anv_video_mfc.c b/src/intel/vulkan_hasvk/anv_video_mfc.c index f1f5600..fbfd65e 100644 --- a/src/intel/vulkan_hasvk/anv_video_mfc.c +++ b/src/intel/vulkan_hasvk/anv_video_mfc.c @@ -192,9 +192,9 @@ genX(hasvk_mfc_emit_static_state)(struct anv_cmd_buffer *cmd_buffer, avc_img.TrellisQuantizationChromaDisable = false; avc_img.IntraMBConformanceMaxSize = 0xEE8; avc_img.InterMBConformanceMaxSize = 0xBB8; - avc_img.NumberofReferenceFrames = is_p ? 1 : 0; - avc_img.NumberofActiveReferencePicturesfromL0 = is_p ? 1 : 0; - avc_img.NumberofActiveReferencePicturesfromL1 = 0; + avc_img.NumberofReferenceFrames = refs->l0_count + refs->l1_count; + avc_img.NumberofActiveReferencePicturesfromL0 = refs->l0_count; + avc_img.NumberofActiveReferencePicturesfromL1 = refs->l1_count; avc_img.InitialQPValue = 0; /* per-slice QP comes from SLICE_STATE */ avc_img.PicOrderPresent = pps->flags.bottom_field_pic_order_in_frame_present_flag; avc_img.DeltaPicOrderAlwaysZero = sps->flags.delta_pic_order_always_zero_flag; @@ -304,11 +304,12 @@ genX(hasvk_mfc_emit_static_state)(struct anv_cmd_buffer *cmd_buffer, anv_batch_emit(&cmd_buffer->batch, GENX(MFX_AVC_REF_IDX_STATE), ref_idx) { ref_idx.ReferencePictureListSelect = list; memset(ref_idx.ReferenceListEntry, 0x80, sizeof(ref_idx.ReferenceListEntry)); - if (is_p && list == 0) { - unsigned n = MIN2(refs->l0_count, - ARRAY_SIZE(ref_idx.ReferenceListEntry)); + const uint8_t *slots = list == 0 ? refs->l0_slot : refs->l1_slot; + unsigned count = list == 0 ? refs->l0_count : refs->l1_count; + if (is_p) { + unsigned n = MIN2(count, ARRAY_SIZE(ref_idx.ReferenceListEntry)); for (unsigned i = 0; i < n; i++) - ref_idx.ReferenceListEntry[i] = 0x20 | (refs->l0_slot[i] << 1); + ref_idx.ReferenceListEntry[i] = 0x20 | (slots[i] << 1); } } } @@ -328,16 +329,19 @@ genX(hasvk_mfc_emit_slice)(struct anv_cmd_buffer *cmd_buffer, uint32_t dst_page_offset, const struct anv_video_intra_refresh *intra_refresh) { - /* A P slice references the previous reconstructed frame; an I slice does - * not. B slices are out of scope. */ - bool is_p = (slice_hdr->slice_type == STD_VIDEO_H264_SLICE_TYPE_P); + /* An I slice predicts from nothing; P and B slices both predict from L0, + * and a B slice additionally has L1 available. */ + bool is_b = (slice_hdr->slice_type == STD_VIDEO_H264_SLICE_TYPE_B); + bool is_p = is_b || (slice_hdr->slice_type == STD_VIDEO_H264_SLICE_TYPE_P); /* MFX_AVC_SLICE_STATE is not used by decode; fields are matched against the * VA-API reference encoder (gen6_mfc_avc_slice_state). The slice covers the * macroblock range [slice_start_mb, slice_end_mb) in raster order. */ anv_batch_emit(&cmd_buffer->batch, GENX(MFX_AVC_SLICE_STATE), slice) { - slice.SliceType = is_p ? PSlice : ISlice; /* P=0,B=1,I=2 */ + slice.SliceType = is_b ? BSlice : + is_p ? PSlice : ISlice; /* P=0,B=1,I=2 */ slice.NumberofReferencePicturesinInterpredictionList0 = is_p ? 1 : 0; + slice.NumberofReferencePicturesinInterpredictionList1 = is_b ? 1 : 0; slice.SliceQuantizationParameter = qp; slice.CABACInitIDC = is_p ? slice_hdr->cabac_init_idc : 0; slice.DisableDeblockingFilterIndicator = 1; diff --git a/src/intel/vulkan_hasvk/genX_cmd_video.c b/src/intel/vulkan_hasvk/genX_cmd_video.c index 68878e7..1f722f0 100644 --- a/src/intel/vulkan_hasvk/genX_cmd_video.c +++ b/src/intel/vulkan_hasvk/genX_cmd_video.c @@ -1237,6 +1237,14 @@ anv_h264_encode_video(struct anv_cmd_buffer *cmd_buffer, break; refs.l0_slot[refs.l0_count++] = s; } + n = MIN2(ref_lists->num_ref_idx_l1_active_minus1 + 1, + ANV_VIDEO_MAX_DPB_SLOTS); + for (unsigned i = 0; i < n; i++) { + uint8_t s = ref_lists->RefPicList1[i]; + if (s >= ANV_VIDEO_MAX_DPB_SLOTS || !refs.slot_valid[s]) + break; + refs.l1_slot[refs.l1_count++] = s; + } } /* A client that supplies reference slots but no list still means the * first one, which is what the encoder used to assume unconditionally. */