[FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Fei Wang]

From: Linjie Fu <linjie.fu@intel.com>

Use GPB frames to replace regular P/B frames if backend driver does not
support it.

- GPB:
    Generalized P and B picture. Regular P/B frames replaced by B
    frames with previous-predict only, L0 == L1. Normal B frames
    still have 2 different ref_lists and allow bi-prediction

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
update:
1. Add b to gpb.
2. Optimise debug message.

 libavcodec/vaapi_encode.c      | 74 +++++++++++++++++++++++++++++++---
 libavcodec/vaapi_encode.h      |  2 +
 libavcodec/vaapi_encode_h265.c | 24 ++++++++++-
 3 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 3bf379b1a0..bdba9726b2 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -848,9 +848,13 @@ static void vaapi_encode_set_b_pictures(AVCodecContext *avctx,
             pic->b_depth = current_depth;
 
             vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
-            vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
             vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
 
+            if (!ctx->b_to_gpb)
+                vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
+            else
+                vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
+
             for (ref = end->refs[1]; ref; ref = ref->refs[1])
                 vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
         }
@@ -871,8 +875,11 @@ static void vaapi_encode_set_b_pictures(AVCodecContext *avctx,
 
         vaapi_encode_add_ref(avctx, pic, pic,   0, 1, 0);
         vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
-        vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
         vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
+        if (!ctx->b_to_gpb)
+            vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
+        else
+            vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
 
         for (ref = end->refs[1]; ref; ref = ref->refs[1])
             vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
@@ -1845,6 +1852,51 @@ static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
         ref_l1 = attr.value >> 16 & 0xffff;
     }
 
+    ctx->p_to_gpb = 0;
+    ctx->b_to_gpb = 0;
+
+#if VA_CHECK_VERSION(1, 9, 0)
+    if (!(ctx->codec->flags & FLAG_INTRA_ONLY ||
+        avctx->gop_size <= 1)) {
+        attr = (VAConfigAttrib) { VAConfigAttribPredictionDirection };
+        vas = vaGetConfigAttributes(ctx->hwctx->display,
+                                    ctx->va_profile,
+                                    ctx->va_entrypoint,
+                                    &attr, 1);
+        if (vas != VA_STATUS_SUCCESS) {
+            av_log(avctx, AV_LOG_WARNING, "Failed to query prediction direction "
+                   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
+            return AVERROR_EXTERNAL;
+        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
+            av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any additional "
+                   "prediction constraints.\n");
+        } else {
+            if (((ref_l0 > 0 || ref_l1 > 0) && !(attr.value & VA_PREDICTION_DIRECTION_PREVIOUS)) ||
+                ((ref_l1 == 0) && (attr.value & (VA_PREDICTION_DIRECTION_FUTURE | VA_PREDICTION_DIRECTION_BI_NOT_EMPTY)))) {
+                av_log(avctx, AV_LOG_ERROR, "Driver report incorrect prediction "
+                       "direction attribute.\n");
+                return AVERROR_EXTERNAL;
+            }
+
+            if (!(attr.value & VA_PREDICTION_DIRECTION_FUTURE)) {
+                if (ref_l0 > 0 && ref_l1 > 0) {
+                    ctx->b_to_gpb = 1;
+                    av_log(avctx, AV_LOG_VERBOSE, "Driver support previous prediction "
+                           "only for B-frames.\n");
+                }
+            }
+
+            if (attr.value & VA_PREDICTION_DIRECTION_BI_NOT_EMPTY) {
+                if (ref_l0 > 0 && ref_l1 > 0) {
+                    ctx->p_to_gpb = 1;
+                    av_log(avctx, AV_LOG_VERBOSE, "Driver does not support P-frames, "
+                           "replacing them with previous prediction only B-frames.\n");
+                }
+            }
+        }
+    }
+#endif
+
     if (ctx->codec->flags & FLAG_INTRA_ONLY ||
         avctx->gop_size <= 1) {
         av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");
@@ -1855,14 +1907,24 @@ static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
         return AVERROR(EINVAL);
     } else if (!(ctx->codec->flags & FLAG_B_PICTURES) ||
                ref_l1 < 1 || avctx->max_b_frames < 1) {
-        av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "
-               "(supported references: %d / %d).\n", ref_l0, ref_l1);
+        if (ctx->p_to_gpb)
+           av_log(avctx, AV_LOG_VERBOSE, "Using intra and B-frames "
+                  "(supported references: %d / %d).\n",
+                  ref_l0, ref_l1);
+        else
+            av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "
+                   "(supported references: %d / %d).\n", ref_l0, ref_l1);
         ctx->gop_size = avctx->gop_size;
         ctx->p_per_i  = INT_MAX;
         ctx->b_per_p  = 0;
     } else {
-        av_log(avctx, AV_LOG_VERBOSE, "Using intra, P- and B-frames "
-               "(supported references: %d / %d).\n", ref_l0, ref_l1);
+       if (ctx->p_to_gpb)
+           av_log(avctx, AV_LOG_VERBOSE, "Using intra and B-frames "
+                  "(supported references: %d / %d).\n",
+                  ref_l0, ref_l1);
+       else
+           av_log(avctx, AV_LOG_VERBOSE, "Using intra, P- and B-frames "
+                  "(supported references: %d / %d).\n", ref_l0, ref_l1);
         ctx->gop_size = avctx->gop_size;
         ctx->p_per_i  = INT_MAX;
         ctx->b_per_p  = avctx->max_b_frames;
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index b41604a883..4ce1df0c6f 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -328,6 +328,8 @@ typedef struct VAAPIEncodeContext {
     int idr_counter;
     int gop_counter;
     int end_of_stream;
+    int p_to_gpb;
+    int b_to_gpb;
 
     // Whether the driver supports ROI at all.
     int             roi_allowed;
diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index e1dc53dfa9..25b9a707d2 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -717,7 +717,10 @@ static int vaapi_encode_h265_init_picture_params(AVCodecContext *avctx,
             hpic->pic_type       = 1;
         } else {
             VAAPIEncodePicture *irap_ref;
-            av_assert0(pic->refs[0] && pic->refs[1]);
+            if (!ctx->b_to_gpb)
+                av_assert0(pic->refs[0] && pic->refs[1]);
+            else
+                av_assert0(pic->refs[0]);
             for (irap_ref = pic; irap_ref; irap_ref = irap_ref->refs[1]) {
                 if (irap_ref->type == PICTURE_TYPE_I)
                     break;
@@ -886,6 +889,7 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
                                                VAAPIEncodePicture *pic,
                                                VAAPIEncodeSlice *slice)
 {
+    VAAPIEncodeContext                *ctx = avctx->priv_data;
     VAAPIEncodeH265Context           *priv = avctx->priv_data;
     VAAPIEncodeH265Picture           *hpic = pic->priv_data;
     const H265RawSPS                  *sps = &priv->raw_sps;
@@ -908,6 +912,9 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
 
     sh->slice_type = hpic->slice_type;
 
+    if (sh->slice_type == HEVC_SLICE_P && ctx->p_to_gpb)
+        sh->slice_type = HEVC_SLICE_B;
+
     sh->slice_pic_order_cnt_lsb = hpic->pic_order_cnt &
         (1 << (sps->log2_max_pic_order_cnt_lsb_minus4 + 4)) - 1;
 
@@ -1066,11 +1073,26 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
         av_assert0(pic->type == PICTURE_TYPE_P ||
                    pic->type == PICTURE_TYPE_B);
         vslice->ref_pic_list0[0] = vpic->reference_frames[0];
+        if (ctx->p_to_gpb && pic->type == PICTURE_TYPE_P)
+            // Reference for GPB B-frame, L0 == L1
+            vslice->ref_pic_list1[0] = vpic->reference_frames[0];
     }
     if (pic->nb_refs >= 2) {
         // Forward reference for B-frame.
         av_assert0(pic->type == PICTURE_TYPE_B);
         vslice->ref_pic_list1[0] = vpic->reference_frames[1];
+        if (ctx->b_to_gpb && pic->type == PICTURE_TYPE_B)
+            // Reference for GPB B-frame, L0 == L1
+            vslice->ref_pic_list1[0] = vpic->reference_frames[0];
+    }
+
+    if ((pic->type == PICTURE_TYPE_P && ctx->p_to_gpb) ||
+        (pic->type == PICTURE_TYPE_B && ctx->b_to_gpb)) {
+        vslice->slice_type = HEVC_SLICE_B;
+        for (i = 0; i < FF_ARRAY_ELEMS(vslice->ref_pic_list0); i++) {
+            vslice->ref_pic_list1[i].picture_id = vslice->ref_pic_list0[i].picture_id;
+            vslice->ref_pic_list1[i].flags      = vslice->ref_pic_list0[i].flags;
+        }
     }
 
     return 0;
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 2/4] vaapi_encode: Move block size calculation after entrypoint selection

[Fei Wang]

From: Mark Thompson <sw@jkqxz.net>

The block size can be dependent on the profile and entrypoint selected.
It defaults to 16x16, with codecs able to override this choice with their
own function.

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
 libavcodec/vaapi_encode.c       | 16 ++++++++++++++++
 libavcodec/vaapi_encode.h       |  7 +++++++
 libavcodec/vaapi_encode_h265.c  | 32 ++++++++++++++++++++++++++------
 libavcodec/vaapi_encode_mjpeg.c | 18 +++++++++++++++---
 libavcodec/vaapi_encode_mpeg2.c |  3 ---
 libavcodec/vaapi_encode_vp8.c   |  3 ---
 libavcodec/vaapi_encode_vp9.c   | 16 ++++++++++++----
 7 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index bdba9726b2..d0aebad681 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -2074,6 +2074,8 @@ static av_cold int vaapi_encode_init_slice_structure(AVCodecContext *avctx)
         return 0;
     }
 
+    av_assert0(ctx->slice_block_height > 0 && ctx->slice_block_width > 0);
+
     ctx->slice_block_rows = (avctx->height + ctx->slice_block_height - 1) /
                              ctx->slice_block_height;
     ctx->slice_block_cols = (avctx->width  + ctx->slice_block_width  - 1) /
@@ -2463,6 +2465,20 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
     if (err < 0)
         goto fail;
 
+    if (ctx->codec->get_encoder_caps) {
+        err = ctx->codec->get_encoder_caps(avctx);
+        if (err < 0)
+            goto fail;
+    } else {
+        // Assume 16x16 blocks.
+        ctx->surface_width  = FFALIGN(avctx->width,  16);
+        ctx->surface_height = FFALIGN(avctx->height, 16);
+        if (ctx->codec->flags & FLAG_SLICE_CONTROL) {
+            ctx->slice_block_width  = 16;
+            ctx->slice_block_height = 16;
+        }
+    }
+
     err = vaapi_encode_init_rate_control(avctx);
     if (err < 0)
         goto fail;
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 4ce1df0c6f..8a0b9a1e48 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -377,6 +377,13 @@ typedef struct VAAPIEncodeType {
     // factor depending on RC mode.
     int default_quality;
 
+    // Determine encode parameters like block sizes for surface alignment
+    // and slices. This may need to query the profile and entrypoint,
+    // which will be available when this function is called. If not set,
+    // assume that all blocks are 16x16 and that surfaces should be
+    // aligned to match this.
+    int (*get_encoder_caps)(AVCodecContext *avctx);
+
     // Perform any extra codec-specific configuration after the
     // codec context is initialised (set up the private data and
     // add any necessary global parameters).
diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 25b9a707d2..099a68cc1b 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -55,6 +55,10 @@ typedef struct VAAPIEncodeH265Picture {
 typedef struct VAAPIEncodeH265Context {
     VAAPIEncodeContext common;
 
+    // Encoder features.
+    uint32_t ctu_size;
+    uint32_t min_cb_size;
+
     // User options.
     int qp;
     int aud;
@@ -1098,6 +1102,27 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
     return 0;
 }
 
+static av_cold int vaapi_encode_h265_get_encoder_caps(AVCodecContext *avctx)
+{
+    VAAPIEncodeContext      *ctx = avctx->priv_data;
+    VAAPIEncodeH265Context *priv = avctx->priv_data;
+
+    if (!priv->ctu_size) {
+        priv->ctu_size     = 32;
+        priv->min_cb_size  = 16;
+    }
+    av_log(avctx, AV_LOG_VERBOSE, "Using CTU size %dx%d, "
+           "min CB size %dx%d.\n", priv->ctu_size, priv->ctu_size,
+           priv->min_cb_size, priv->min_cb_size);
+
+    ctx->surface_width  = FFALIGN(avctx->width,  priv->min_cb_size);
+    ctx->surface_height = FFALIGN(avctx->height, priv->min_cb_size);
+
+    ctx->slice_block_width = ctx->slice_block_height = priv->ctu_size;
+
+    return 0;
+}
+
 static av_cold int vaapi_encode_h265_configure(AVCodecContext *avctx)
 {
     VAAPIEncodeContext      *ctx = avctx->priv_data;
@@ -1167,6 +1192,7 @@ static const VAAPIEncodeType vaapi_encode_type_h265 = {
 
     .default_quality       = 25,
 
+    .get_encoder_caps      = &vaapi_encode_h265_get_encoder_caps,
     .configure             = &vaapi_encode_h265_configure,
 
     .picture_priv_data_size = sizeof(VAAPIEncodeH265Picture),
@@ -1212,12 +1238,6 @@ static av_cold int vaapi_encode_h265_init(AVCodecContext *avctx)
         VA_ENC_PACKED_HEADER_SLICE    | // Slice headers.
         VA_ENC_PACKED_HEADER_MISC;      // SEI
 
-    ctx->surface_width  = FFALIGN(avctx->width,  16);
-    ctx->surface_height = FFALIGN(avctx->height, 16);
-
-    // CTU size is currently hard-coded to 32.
-    ctx->slice_block_width = ctx->slice_block_height = 32;
-
     if (priv->qp > 0)
         ctx->explicit_qp = priv->qp;
 
diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c
index 6206b23e5f..87a03f181b 100644
--- a/libavcodec/vaapi_encode_mjpeg.c
+++ b/libavcodec/vaapi_encode_mjpeg.c
@@ -434,6 +434,20 @@ static int vaapi_encode_mjpeg_init_slice_params(AVCodecContext *avctx,
     return 0;
 }
 
+static av_cold int vaapi_encode_mjpeg_get_encoder_caps(AVCodecContext *avctx)
+{
+    VAAPIEncodeContext *ctx = avctx->priv_data;
+    const AVPixFmtDescriptor *desc;
+
+    desc = av_pix_fmt_desc_get(ctx->input_frames->sw_format);
+    av_assert0(desc);
+
+    ctx->surface_width  = FFALIGN(avctx->width,  8 << desc->log2_chroma_w);
+    ctx->surface_height = FFALIGN(avctx->height, 8 << desc->log2_chroma_h);
+
+    return 0;
+}
+
 static av_cold int vaapi_encode_mjpeg_configure(AVCodecContext *avctx)
 {
     VAAPIEncodeContext       *ctx = avctx->priv_data;
@@ -483,6 +497,7 @@ static const VAAPIEncodeType vaapi_encode_type_mjpeg = {
     .flags                 = FLAG_CONSTANT_QUALITY_ONLY |
                              FLAG_INTRA_ONLY,
 
+    .get_encoder_caps      = &vaapi_encode_mjpeg_get_encoder_caps,
     .configure             = &vaapi_encode_mjpeg_configure,
 
     .default_quality       = 80,
@@ -509,9 +524,6 @@ static av_cold int vaapi_encode_mjpeg_init(AVCodecContext *avctx)
     ctx->desired_packed_headers =
         VA_ENC_PACKED_HEADER_RAW_DATA;
 
-    ctx->surface_width  = FFALIGN(avctx->width,  8);
-    ctx->surface_height = FFALIGN(avctx->height, 8);
-
     return ff_vaapi_encode_init(avctx);
 }
 
diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c
index d084d8c4b9..f8809b08ab 100644
--- a/libavcodec/vaapi_encode_mpeg2.c
+++ b/libavcodec/vaapi_encode_mpeg2.c
@@ -623,9 +623,6 @@ static av_cold int vaapi_encode_mpeg2_init(AVCodecContext *avctx)
     ctx->desired_packed_headers = VA_ENC_PACKED_HEADER_SEQUENCE |
                                   VA_ENC_PACKED_HEADER_PICTURE;
 
-    ctx->surface_width  = FFALIGN(avctx->width,  16);
-    ctx->surface_height = FFALIGN(avctx->height, 16);
-
     return ff_vaapi_encode_init(avctx);
 }
 
diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c
index 7d4578f674..ab3c84a618 100644
--- a/libavcodec/vaapi_encode_vp8.c
+++ b/libavcodec/vaapi_encode_vp8.c
@@ -210,9 +210,6 @@ static av_cold int vaapi_encode_vp8_init(AVCodecContext *avctx)
     // adding them anyway.
     ctx->desired_packed_headers = 0;
 
-    ctx->surface_width  = FFALIGN(avctx->width,  16);
-    ctx->surface_height = FFALIGN(avctx->height, 16);
-
     return ff_vaapi_encode_init(avctx);
 }
 
diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c
index be706e3bd6..837adaa825 100644
--- a/libavcodec/vaapi_encode_vp9.c
+++ b/libavcodec/vaapi_encode_vp9.c
@@ -184,6 +184,17 @@ static int vaapi_encode_vp9_init_picture_params(AVCodecContext *avctx,
     return 0;
 }
 
+static av_cold int vaapi_encode_vp9_get_encoder_caps(AVCodecContext *avctx)
+{
+    VAAPIEncodeContext *ctx = avctx->priv_data;
+
+    // Surfaces must be aligned to 64x64 superblock boundaries.
+    ctx->surface_width  = FFALIGN(avctx->width,  64);
+    ctx->surface_height = FFALIGN(avctx->height, 64);
+
+    return 0;
+}
+
 static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx)
 {
     VAAPIEncodeContext     *ctx = avctx->priv_data;
@@ -231,6 +242,7 @@ static const VAAPIEncodeType vaapi_encode_type_vp9 = {
 
     .picture_priv_data_size = sizeof(VAAPIEncodeVP9Picture),
 
+    .get_encoder_caps      = &vaapi_encode_vp9_get_encoder_caps,
     .configure             = &vaapi_encode_vp9_configure,
 
     .sequence_params_size  = sizeof(VAEncSequenceParameterBufferVP9),
@@ -251,10 +263,6 @@ static av_cold int vaapi_encode_vp9_init(AVCodecContext *avctx)
     // can write its own headers and there is no metadata to include.
     ctx->desired_packed_headers = 0;
 
-    // Surfaces must be aligned to superblock boundaries.
-    ctx->surface_width  = FFALIGN(avctx->width,  64);
-    ctx->surface_height = FFALIGN(avctx->height, 64);
-
     return ff_vaapi_encode_init(avctx);
 }
 
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 3/4] vaapi_encode_h265: Explicitly set and correct some flags

[Fei Wang]

From: Mark Thompson <sw@jkqxz.net>

max_14bit_constraint_flag should be set if the bit depth is not greater than
14 (currently always true).

one_picture_only_flag should not be set because we don't support the still
picture profiles.

general_profile_compatibility_flag should be set according to general_profile_idc
instead of bit depth.

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
 libavcodec/vaapi_encode_h265.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 099a68cc1b..e486e64eb9 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -303,17 +303,21 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
     ptl->general_profile_idc   = avctx->profile;
     ptl->general_tier_flag     = priv->tier;
 
-    if (chroma_format == 1) {
-        ptl->general_profile_compatibility_flag[1] = bit_depth ==  8;
-        ptl->general_profile_compatibility_flag[2] = bit_depth <= 10;
+    ptl->general_profile_compatibility_flag[ptl->general_profile_idc] = 1;
+
+    if (ptl->general_profile_compatibility_flag[1])
+        ptl->general_profile_compatibility_flag[2] = 1;
+    if (ptl->general_profile_compatibility_flag[3]) {
+        ptl->general_profile_compatibility_flag[1] = 1;
+        ptl->general_profile_compatibility_flag[2] = 1;
     }
-    ptl->general_profile_compatibility_flag[4] = 1;
 
     ptl->general_progressive_source_flag    = 1;
     ptl->general_interlaced_source_flag     = 0;
     ptl->general_non_packed_constraint_flag = 1;
     ptl->general_frame_only_constraint_flag = 1;
 
+    ptl->general_max_14bit_constraint_flag = bit_depth <= 14;
     ptl->general_max_12bit_constraint_flag = bit_depth <= 12;
     ptl->general_max_10bit_constraint_flag = bit_depth <= 10;
     ptl->general_max_8bit_constraint_flag  = bit_depth ==  8;
@@ -323,6 +327,7 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
     ptl->general_max_monochrome_constraint_flag = chroma_format == 0;
 
     ptl->general_intra_constraint_flag = ctx->gop_size == 1;
+    ptl->general_one_picture_only_constraint_flag = 0;
 
     ptl->general_lower_bit_rate_constraint_flag = 1;
 
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 4/4] vaapi_encode_h265: Query encoding block sizes and features

[Fei Wang]

From: Mark Thompson <sw@jkqxz.net>

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
 libavcodec/vaapi_encode_h265.c | 114 +++++++++++++++++++++++++++++++--
 1 file changed, 108 insertions(+), 6 deletions(-)

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index e486e64eb9..11eca5989b 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -56,6 +56,9 @@ typedef struct VAAPIEncodeH265Context {
     VAAPIEncodeContext common;
 
     // Encoder features.
+    uint32_t va_features;
+    // Block size info.
+    uint32_t va_bs;
     uint32_t ctu_size;
     uint32_t min_cb_size;
 
@@ -427,9 +430,9 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
             vps->vps_max_latency_increase_plus1[i];
     }
 
-    // These have to come from the capabilities of the encoder.  We have no
-    // way to query them, so just hardcode parameters which work on the Intel
-    // driver.
+    // These values come from the capabilities of the first encoder
+    // implementation in the i965 driver on Intel Skylake.  They may
+    // fail badly with other platforms or drivers.
     // CTB size from 8x8 to 32x32.
     sps->log2_min_luma_coding_block_size_minus3   = 0;
     sps->log2_diff_max_min_luma_coding_block_size = 2;
@@ -447,6 +450,42 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
 
     sps->pcm_enabled_flag = 0;
 
+// update sps setting according to queried result
+#if VA_CHECK_VERSION(1, 13, 0)
+    if (priv->va_features) {
+        VAConfigAttribValEncHEVCFeatures features = { .value = priv->va_features };
+
+        // Enable feature if get queried result is VA_FEATURE_SUPPORTED | VA_FEATURE_REQUIRED
+        sps->amp_enabled_flag =
+            !!features.bits.amp;
+        sps->sample_adaptive_offset_enabled_flag =
+            !!features.bits.sao;
+        sps->sps_temporal_mvp_enabled_flag =
+            !!features.bits.temporal_mvp;
+        sps->pcm_enabled_flag =
+            !!features.bits.pcm;
+    }
+
+    if (priv->va_bs) {
+        VAConfigAttribValEncHEVCBlockSizes bs = { .value = priv->va_bs };
+        sps->log2_min_luma_coding_block_size_minus3 =
+            ff_ctz(priv->min_cb_size) - 3;
+        sps->log2_diff_max_min_luma_coding_block_size =
+            ff_ctz(priv->ctu_size) - ff_ctz(priv->min_cb_size);
+
+        sps->log2_min_luma_transform_block_size_minus2 =
+            bs.bits.log2_min_luma_transform_block_size_minus2;
+        sps->log2_diff_max_min_luma_transform_block_size =
+            bs.bits.log2_max_luma_transform_block_size_minus2 -
+            bs.bits.log2_min_luma_transform_block_size_minus2;
+
+        sps->max_transform_hierarchy_depth_inter =
+            bs.bits.max_max_transform_hierarchy_depth_inter;
+        sps->max_transform_hierarchy_depth_intra =
+            bs.bits.max_max_transform_hierarchy_depth_intra;
+    }
+#endif
+
     // STRPSs should ideally be here rather than defined individually in
     // each slice, but the structure isn't completely fixed so for now
     // don't bother.
@@ -539,6 +578,23 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
     pps->cu_qp_delta_enabled_flag = (ctx->va_rc_mode != VA_RC_CQP);
     pps->diff_cu_qp_delta_depth   = 0;
 
+// update pps setting according to queried result
+#if VA_CHECK_VERSION(1, 13, 0)
+    if (priv->va_features) {
+        VAConfigAttribValEncHEVCFeatures features = { .value = priv->va_features };
+        if (ctx->va_rc_mode != VA_RC_CQP)
+            pps->cu_qp_delta_enabled_flag =
+                !!features.bits.cu_qp_delta;
+
+        pps->transform_skip_enabled_flag =
+            !!features.bits.transform_skip;
+        // set diff_cu_qp_delta_depth as its max value if cu_qp_delta enabled. Otherwise
+        // 0 will make cu_qp_delta invalid.
+        if (pps->cu_qp_delta_enabled_flag)
+            pps->diff_cu_qp_delta_depth = sps->log2_diff_max_min_luma_coding_block_size;
+    }
+#endif
+
     if (ctx->tile_rows && ctx->tile_cols) {
         int uniform_spacing;
 
@@ -640,8 +696,8 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
 
         .coded_buf = VA_INVALID_ID,
 
-        .collocated_ref_pic_index = 0xff,
-
+        .collocated_ref_pic_index = sps->sps_temporal_mvp_enabled_flag ?
+                                    0 : 0xff,
         .last_picture = 0,
 
         .pic_init_qp            = pps->init_qp_minus26 + 26,
@@ -674,6 +730,8 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
             .entropy_coding_sync_enabled_flag = pps->entropy_coding_sync_enabled_flag,
             .loop_filter_across_tiles_enabled_flag =
                 pps->loop_filter_across_tiles_enabled_flag,
+            .pps_loop_filter_across_slices_enabled_flag =
+                pps->pps_loop_filter_across_slices_enabled_flag,
             .scaling_list_data_present_flag = (sps->sps_scaling_list_data_present_flag |
                                                pps->pps_scaling_list_data_present_flag),
             .screen_content_flag            = 0,
@@ -1004,10 +1062,13 @@ static int vaapi_encode_h265_init_slice_params(AVCodecContext *avctx,
         sh->num_long_term_sps  = 0;
         sh->num_long_term_pics = 0;
 
+        // when this flag is not present, it is inerred to 1.
+        sh->collocated_from_l0_flag = 1;
         sh->slice_temporal_mvp_enabled_flag =
             sps->sps_temporal_mvp_enabled_flag;
         if (sh->slice_temporal_mvp_enabled_flag) {
-            sh->collocated_from_l0_flag = sh->slice_type == HEVC_SLICE_B;
+            if (sh->slice_type == HEVC_SLICE_B)
+                sh->collocated_from_l0_flag = 1;
             sh->collocated_ref_idx      = 0;
         }
 
@@ -1112,6 +1173,47 @@ static av_cold int vaapi_encode_h265_get_encoder_caps(AVCodecContext *avctx)
     VAAPIEncodeContext      *ctx = avctx->priv_data;
     VAAPIEncodeH265Context *priv = avctx->priv_data;
 
+#if VA_CHECK_VERSION(1, 13, 0)
+    {
+        VAConfigAttribValEncHEVCBlockSizes block_size;
+        VAConfigAttrib attr;
+        VAStatus vas;
+
+        attr.type = VAConfigAttribEncHEVCFeatures;
+        vas = vaGetConfigAttributes(ctx->hwctx->display, ctx->va_profile,
+                                    ctx->va_entrypoint, &attr, 1);
+        if (vas != VA_STATUS_SUCCESS) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to query encoder "
+                   "features, using guessed defaults.\n");
+            return AVERROR_EXTERNAL;
+        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
+            av_log(avctx, AV_LOG_WARNING, "Driver does not advertise "
+                   "encoder features, using guessed defaults.\n");
+        } else {
+            priv->va_features = attr.value;
+        }
+
+        attr.type = VAConfigAttribEncHEVCBlockSizes;
+        vas = vaGetConfigAttributes(ctx->hwctx->display, ctx->va_profile,
+                                    ctx->va_entrypoint, &attr, 1);
+        if (vas != VA_STATUS_SUCCESS) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to query encoder "
+                   "block size, using guessed defaults.\n");
+            return AVERROR_EXTERNAL;
+        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
+            av_log(avctx, AV_LOG_WARNING, "Driver does not advertise "
+                   "encoder block size, using guessed defaults.\n");
+        } else {
+            priv->va_bs = block_size.value = attr.value;
+
+            priv->ctu_size =
+                1 << block_size.bits.log2_max_coding_tree_block_size_minus3 + 3;
+            priv->min_cb_size =
+                1 << block_size.bits.log2_min_luma_coding_block_size_minus3 + 3;
+        }
+    }
+#endif
+
     if (!priv->ctu_size) {
         priv->ctu_size     = 32;
         priv->min_cb_size  = 16;
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Mark Thompson]

On 11/03/2022 09:00, Fei Wang wrote:
> From: Linjie Fu <linjie.fu@intel.com>
> 
> Use GPB frames to replace regular P/B frames if backend driver does not
> support it.
> 
> - GPB:
>      Generalized P and B picture. Regular P/B frames replaced by B
>      frames with previous-predict only, L0 == L1. Normal B frames
>      still have 2 different ref_lists and allow bi-prediction
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> ---
> update:
> 1. Add b to gpb.
> 2. Optimise debug message.
> 
>   libavcodec/vaapi_encode.c      | 74 +++++++++++++++++++++++++++++++---
>   libavcodec/vaapi_encode.h      |  2 +
>   libavcodec/vaapi_encode_h265.c | 24 ++++++++++-
>   3 files changed, 93 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 3bf379b1a0..bdba9726b2 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -848,9 +848,13 @@ static void vaapi_encode_set_b_pictures(AVCodecContext *avctx,
>               pic->b_depth = current_depth;
>   
>               vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
> -            vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
>               vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
>   
> +            if (!ctx->b_to_gpb)
> +                vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
> +            else
> +                vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);

This is doing something extremely dubious.  If b-to-gpb is set, then don't use the future reference?  That means these pictures will only have the past reference, and the coding efficiency will often be much worse.

E.g. if you have the default structure (max_b_frames = 2, max_b_depth = 1) then in a sequence of four pictures you get:

1 referring to something previous
4 referring to 1
2 referring to 1 and 4
3 referring to 1 and 4

and this change means you lose the 2 -> 4 and 3 -> 4 references.  Therefore, a change in the picture between 1 and 2 will end up coded three times in 2, 3 and 4 rather than just being coded in 4 and then referred to by the others.

> +
>               for (ref = end->refs[1]; ref; ref = ref->refs[1])
>                   vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
>           }
> @@ -871,8 +875,11 @@ static void vaapi_encode_set_b_pictures(AVCodecContext *avctx,
>   
>           vaapi_encode_add_ref(avctx, pic, pic,   0, 1, 0);
>           vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
> -        vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
>           vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
> +        if (!ctx->b_to_gpb)
> +            vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
> +        else
> +            vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
>   
>           for (ref = end->refs[1]; ref; ref = ref->refs[1])
>               vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
> @@ -1845,6 +1852,51 @@ static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>           ref_l1 = attr.value >> 16 & 0xffff;
>       }
>   
> +    ctx->p_to_gpb = 0;
> +    ctx->b_to_gpb = 0;
> +
> +#if VA_CHECK_VERSION(1, 9, 0)
> +    if (!(ctx->codec->flags & FLAG_INTRA_ONLY ||
> +        avctx->gop_size <= 1)) {
> +        attr = (VAConfigAttrib) { VAConfigAttribPredictionDirection };
> +        vas = vaGetConfigAttributes(ctx->hwctx->display,
> +                                    ctx->va_profile,
> +                                    ctx->va_entrypoint,
> +                                    &attr, 1);
> +        if (vas != VA_STATUS_SUCCESS) {
> +            av_log(avctx, AV_LOG_WARNING, "Failed to query prediction direction "
> +                   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
> +            return AVERROR_EXTERNAL;
> +        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
> +            av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any additional "
> +                   "prediction constraints.\n");
> +        } else {
> +            if (((ref_l0 > 0 || ref_l1 > 0) && !(attr.value & VA_PREDICTION_DIRECTION_PREVIOUS)) ||
> +                ((ref_l1 == 0) && (attr.value & (VA_PREDICTION_DIRECTION_FUTURE | VA_PREDICTION_DIRECTION_BI_NOT_EMPTY)))) {
> +                av_log(avctx, AV_LOG_ERROR, "Driver report incorrect prediction "
> +                       "direction attribute.\n");
> +                return AVERROR_EXTERNAL;
> +            }
> +
> +            if (!(attr.value & VA_PREDICTION_DIRECTION_FUTURE)) {
> +                if (ref_l0 > 0 && ref_l1 > 0) {
> +                    ctx->b_to_gpb = 1;
> +                    av_log(avctx, AV_LOG_VERBOSE, "Driver support previous prediction "
> +                           "only for B-frames.\n");
> +                }
> +            }

Please note that the PREDICTION_DIRECTION_FUTURE/PREVIOUS options don't mean anything for H.265.

The driver isn't told which direction the prediction is in, it's only told about some reference frames and how to refer to them.  Whether those frames are in the past or future is unspecified and irrelevant - a P frame can refer to a single future frame if it wants.

(I tried to argue this when it was added, but given that they are meaningless I didn't argue very hard.)

I suspect you are trying to use this as a test of something else.  Perhaps you could explain what the test you actually want is?

> +
> +            if (attr.value & VA_PREDICTION_DIRECTION_BI_NOT_EMPTY) {
> +                if (ref_l0 > 0 && ref_l1 > 0) {
> +                    ctx->p_to_gpb = 1;
> +                    av_log(avctx, AV_LOG_VERBOSE, "Driver does not support P-frames, "
> +                           "replacing them with previous prediction only B-frames.\n");
> +                }
> +            }
> +        }
> +    }
> +#endif
> ...

- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Xiang, Haihao]

On Sun, 2022-03-13 at 20:45 +0000, Mark Thompson wrote:
> On 11/03/2022 09:00, Fei Wang wrote:
> > From: Linjie Fu <linjie.fu@intel.com>
> > 
> > Use GPB frames to replace regular P/B frames if backend driver does not
> > support it.
> > 
> > - GPB:
> >      Generalized P and B picture. Regular P/B frames replaced by B
> >      frames with previous-predict only, L0 == L1. Normal B frames
> >      still have 2 different ref_lists and allow bi-prediction
> > 
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> > ---
> > update:
> > 1. Add b to gpb.
> > 2. Optimise debug message.
> > 
> >   libavcodec/vaapi_encode.c      | 74 +++++++++++++++++++++++++++++++---
> >   libavcodec/vaapi_encode.h      |  2 +
> >   libavcodec/vaapi_encode_h265.c | 24 ++++++++++-
> >   3 files changed, 93 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> > index 3bf379b1a0..bdba9726b2 100644
> > --- a/libavcodec/vaapi_encode.c
> > +++ b/libavcodec/vaapi_encode.c
> > @@ -848,9 +848,13 @@ static void vaapi_encode_set_b_pictures(AVCodecContext
> > *avctx,
> >               pic->b_depth = current_depth;
> >   
> >               vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
> > -            vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
> >               vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
> >   
> > +            if (!ctx->b_to_gpb)
> > +                vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
> > +            else
> > +                vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
> 
> This is doing something extremely dubious.  If b-to-gpb is set, then don't use
> the future reference?  

According to 
https://github.com/intel/media-driver/blob/master/media_driver/agnostic/common/codec/hal/codechal_vdenc_hevc.cpp#L3072-L3087
, L0 and L1 should be the same for vdenc hevc on some platforms, so user can't
use past and future reference together, which is why you experienced the failure
after applying version 2 

Thanks
Haihao


> That means these pictures will only have the past reference, and the coding
> efficiency will often be much worse.
> 
> E.g. if you have the default structure (max_b_frames = 2, max_b_depth = 1)
> then in a sequence of four pictures you get:
> 
> 1 referring to something previous
> 4 referring to 1
> 2 referring to 1 and 4
> 3 referring to 1 and 4
> 
> and this change means you lose the 2 -> 4 and 3 -> 4 references.  Therefore, a
> change in the picture between 1 and 2 will end up coded three times in 2, 3
> and 4 rather than just being coded in 4 and then referred to by the others.
> 
> > +
> >               for (ref = end->refs[1]; ref; ref = ref->refs[1])
> >                   vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
> >           }
> > @@ -871,8 +875,11 @@ static void vaapi_encode_set_b_pictures(AVCodecContext
> > *avctx,
> >   
> >           vaapi_encode_add_ref(avctx, pic, pic,   0, 1, 0);
> >           vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
> > -        vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
> >           vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
> > +        if (!ctx->b_to_gpb)
> > +            vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
> > +        else
> > +            vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
> >   
> >           for (ref = end->refs[1]; ref; ref = ref->refs[1])
> >               vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
> > @@ -1845,6 +1852,51 @@ static av_cold int
> > vaapi_encode_init_gop_structure(AVCodecContext *avctx)
> >           ref_l1 = attr.value >> 16 & 0xffff;
> >       }
> >   
> > +    ctx->p_to_gpb = 0;
> > +    ctx->b_to_gpb = 0;
> > +
> > +#if VA_CHECK_VERSION(1, 9, 0)
> > +    if (!(ctx->codec->flags & FLAG_INTRA_ONLY ||
> > +        avctx->gop_size <= 1)) {
> > +        attr = (VAConfigAttrib) { VAConfigAttribPredictionDirection };
> > +        vas = vaGetConfigAttributes(ctx->hwctx->display,
> > +                                    ctx->va_profile,
> > +                                    ctx->va_entrypoint,
> > +                                    &attr, 1);
> > +        if (vas != VA_STATUS_SUCCESS) {
> > +            av_log(avctx, AV_LOG_WARNING, "Failed to query prediction
> > direction "
> > +                   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
> > +            return AVERROR_EXTERNAL;
> > +        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
> > +            av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any
> > additional "
> > +                   "prediction constraints.\n");
> > +        } else {
> > +            if (((ref_l0 > 0 || ref_l1 > 0) && !(attr.value &
> > VA_PREDICTION_DIRECTION_PREVIOUS)) ||
> > +                ((ref_l1 == 0) && (attr.value &
> > (VA_PREDICTION_DIRECTION_FUTURE | VA_PREDICTION_DIRECTION_BI_NOT_EMPTY)))) {
> > +                av_log(avctx, AV_LOG_ERROR, "Driver report incorrect
> > prediction "
> > +                       "direction attribute.\n");
> > +                return AVERROR_EXTERNAL;
> > +            }
> > +
> > +            if (!(attr.value & VA_PREDICTION_DIRECTION_FUTURE)) {
> > +                if (ref_l0 > 0 && ref_l1 > 0) {
> > +                    ctx->b_to_gpb = 1;
> > +                    av_log(avctx, AV_LOG_VERBOSE, "Driver support previous
> > prediction "
> > +                           "only for B-frames.\n");
> > +                }
> > +            }
> 
> Please note that the PREDICTION_DIRECTION_FUTURE/PREVIOUS options don't mean
> anything for H.265.
> 
> The driver isn't told which direction the prediction is in, it's only told
> about some reference frames and how to refer to them.  Whether those frames
> are in the past or future is unspecified and irrelevant - a P frame can refer
> to a single future frame if it wants.
> 
> (I tried to argue this when it was added, but given that they are meaningless
> I didn't argue very hard.)
> 
> I suspect you are trying to use this as a test of something else.  Perhaps you
> could explain what the test you actually want is?
> 
> > +
> > +            if (attr.value & VA_PREDICTION_DIRECTION_BI_NOT_EMPTY) {
> > +                if (ref_l0 > 0 && ref_l1 > 0) {
> > +                    ctx->p_to_gpb = 1;
> > +                    av_log(avctx, AV_LOG_VERBOSE, "Driver does not support
> > P-frames, "
> > +                           "replacing them with previous prediction only B-
> > frames.\n");
> > +                }
> > +            }
> > +        }
> > +    }
> > +#endif
> > ...
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Wang, Fei W]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Xiang,
> Haihao
> Sent: Monday, March 14, 2022 10:15 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB
> frame support for hevc_vaapi
> 
> On Sun, 2022-03-13 at 20:45 +0000, Mark Thompson wrote:
> > On 11/03/2022 09:00, Fei Wang wrote:
> > > From: Linjie Fu <linjie.fu@intel.com>
> > >
> > > Use GPB frames to replace regular P/B frames if backend driver does
> > > not support it.
> > >
> > > - GPB:
> > >      Generalized P and B picture. Regular P/B frames replaced by B
> > >      frames with previous-predict only, L0 == L1. Normal B frames
> > >      still have 2 different ref_lists and allow bi-prediction
> > >
> > > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > > Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> > > ---
> > > update:
> > > 1. Add b to gpb.
> > > 2. Optimise debug message.
> > >
> > >   libavcodec/vaapi_encode.c      | 74 +++++++++++++++++++++++++++++++--
> -
> > >   libavcodec/vaapi_encode.h      |  2 +
> > >   libavcodec/vaapi_encode_h265.c | 24 ++++++++++-
> > >   3 files changed, 93 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> > > index 3bf379b1a0..bdba9726b2 100644
> > > --- a/libavcodec/vaapi_encode.c
> > > +++ b/libavcodec/vaapi_encode.c
> > > @@ -848,9 +848,13 @@ static void
> > > vaapi_encode_set_b_pictures(AVCodecContext
> > > *avctx,
> > >               pic->b_depth = current_depth;
> > >
> > >               vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
> > > -            vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
> > >               vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
> > >
> > > +            if (!ctx->b_to_gpb)
> > > +                vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
> > > +            else
> > > +                vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
> >
> > This is doing something extremely dubious.  If b-to-gpb is set, then
> > don't use the future reference?
> 
> According to
> https://github.com/intel/media-
> driver/blob/master/media_driver/agnostic/common/codec/hal/codechal_vdenc
> _hevc.cpp#L3072-L3087
> , L0 and L1 should be the same for vdenc hevc on some platforms, so user can't
> use past and future reference together, which is why you experienced the failure
> after applying version 2
> 
> Thanks
> Haihao
> 
> 
> > That means these pictures will only have the past reference, and the
> > coding efficiency will often be much worse.
> >
> > E.g. if you have the default structure (max_b_frames = 2, max_b_depth
> > = 1) then in a sequence of four pictures you get:
> >
> > 1 referring to something previous
> > 4 referring to 1
> > 2 referring to 1 and 4
> > 3 referring to 1 and 4
> >
> > and this change means you lose the 2 -> 4 and 3 -> 4 references.
> > Therefore, a change in the picture between 1 and 2 will end up coded
> > three times in 2, 3 and 4 rather than just being coded in 4 and then referred to
> by the others.

If driver doesn't support B frames with two different reference lists, use gpb instead
of regular B is a best way. In V3, I turned B frames to P, but this will break gop structure.
If user set I/P/B frames, while the output only contains I/P frames will be much confuse.

> >
> > > +
> > >               for (ref = end->refs[1]; ref; ref = ref->refs[1])
> > >                   vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
> > >           }
> > > @@ -871,8 +875,11 @@ static void
> > > vaapi_encode_set_b_pictures(AVCodecContext
> > > *avctx,
> > >
> > >           vaapi_encode_add_ref(avctx, pic, pic,   0, 1, 0);
> > >           vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
> > > -        vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
> > >           vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
> > > +        if (!ctx->b_to_gpb)
> > > +            vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
> > > +        else
> > > +            vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
> > >
> > >           for (ref = end->refs[1]; ref; ref = ref->refs[1])
> > >               vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0); @@
> > > -1845,6 +1852,51 @@ static av_cold int
> > > vaapi_encode_init_gop_structure(AVCodecContext *avctx)
> > >           ref_l1 = attr.value >> 16 & 0xffff;
> > >       }
> > >
> > > +    ctx->p_to_gpb = 0;
> > > +    ctx->b_to_gpb = 0;
> > > +
> > > +#if VA_CHECK_VERSION(1, 9, 0)
> > > +    if (!(ctx->codec->flags & FLAG_INTRA_ONLY ||
> > > +        avctx->gop_size <= 1)) {
> > > +        attr = (VAConfigAttrib) { VAConfigAttribPredictionDirection };
> > > +        vas = vaGetConfigAttributes(ctx->hwctx->display,
> > > +                                    ctx->va_profile,
> > > +                                    ctx->va_entrypoint,
> > > +                                    &attr, 1);
> > > +        if (vas != VA_STATUS_SUCCESS) {
> > > +            av_log(avctx, AV_LOG_WARNING, "Failed to query
> > > +prediction
> > > direction "
> > > +                   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
> > > +            return AVERROR_EXTERNAL;
> > > +        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
> > > +            av_log(avctx, AV_LOG_VERBOSE, "Driver does not report
> > > + any
> > > additional "
> > > +                   "prediction constraints.\n");
> > > +        } else {
> > > +            if (((ref_l0 > 0 || ref_l1 > 0) && !(attr.value &
> > > VA_PREDICTION_DIRECTION_PREVIOUS)) ||
> > > +                ((ref_l1 == 0) && (attr.value &
> > > (VA_PREDICTION_DIRECTION_FUTURE |
> > > VA_PREDICTION_DIRECTION_BI_NOT_EMPTY)))) {
> > > +                av_log(avctx, AV_LOG_ERROR, "Driver report
> > > + incorrect
> > > prediction "
> > > +                       "direction attribute.\n");
> > > +                return AVERROR_EXTERNAL;
> > > +            }
> > > +
> > > +            if (!(attr.value & VA_PREDICTION_DIRECTION_FUTURE)) {
> > > +                if (ref_l0 > 0 && ref_l1 > 0) {
> > > +                    ctx->b_to_gpb = 1;
> > > +                    av_log(avctx, AV_LOG_VERBOSE, "Driver support
> > > + previous
> > > prediction "
> > > +                           "only for B-frames.\n");
> > > +                }
> > > +            }
> >
> > Please note that the PREDICTION_DIRECTION_FUTURE/PREVIOUS options
> > don't mean anything for H.265.
> >
> > The driver isn't told which direction the prediction is in, it's only
> > told about some reference frames and how to refer to them.  Whether
> > those frames are in the past or future is unspecified and irrelevant -
> > a P frame can refer to a single future frame if it wants.
> >
> > (I tried to argue this when it was added, but given that they are
> > meaningless I didn't argue very hard.)
> >
> > I suspect you are trying to use this as a test of something else.
> > Perhaps you could explain what the test you actually want is?

VA_PREDICTION_DIRECTION_PREVIOUS/ VA_PREDICTION_DIRECTION_FUTURE/ VA_PREDICTION_DIRECTION_BI_NOT_EMPTY
 are used to indicate if driver has the limitation on how to set regular P and B frame's reference list when the queried max
reference list ref_l0 and ref_l1 both are not zero.

If queried value is VA_PREDICTION_DIRECTION_PREVIOUS only, this means driver doesn't support B frame with different l0/l1,
need to set l1 = 10.
VA_PREDICTION_DIRECTION_PREVIOUS | VA_PREDICTION_DIRECTION_FUTURE means different l0/l1 is support for B frame.

And if queried value is VA_PREDICTION_DIRECTION_BI_NOT_EMPTY, this means driver doesn't support P frame with l0 only.

And in debug message, maybe use "Driver only support same reference list for B-frames\n" will be more clear.

Fei

> >
> > > +
> > > +            if (attr.value & VA_PREDICTION_DIRECTION_BI_NOT_EMPTY) {
> > > +                if (ref_l0 > 0 && ref_l1 > 0) {
> > > +                    ctx->p_to_gpb = 1;
> > > +                    av_log(avctx, AV_LOG_VERBOSE, "Driver does not
> > > + support
> > > P-frames, "
> > > +                           "replacing them with previous prediction
> > > + only B-
> > > frames.\n");
> > > +                }
> > > +            }
> > > +        }
> > > +    }
> > > +#endif
> > > ...
> >
> > - Mark
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Mark Thompson]

On 14/03/2022 11:07, Wang, Fei W wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Xiang,
>> Haihao
>> Sent: Monday, March 14, 2022 10:15 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB
>> frame support for hevc_vaapi
>>
>> On Sun, 2022-03-13 at 20:45 +0000, Mark Thompson wrote:
>>> On 11/03/2022 09:00, Fei Wang wrote:
>>>> From: Linjie Fu <linjie.fu@intel.com>
>>>>
>>>> Use GPB frames to replace regular P/B frames if backend driver does
>>>> not support it.
>>>>
>>>> - GPB:
>>>>       Generalized P and B picture. Regular P/B frames replaced by B
>>>>       frames with previous-predict only, L0 == L1. Normal B frames
>>>>       still have 2 different ref_lists and allow bi-prediction
>>>>
>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
>>>> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
>>>> ---
>>>> update:
>>>> 1. Add b to gpb.
>>>> 2. Optimise debug message.
>>>>
>>>>    libavcodec/vaapi_encode.c      | 74 +++++++++++++++++++++++++++++++--
>> -
>>>>    libavcodec/vaapi_encode.h      |  2 +
>>>>    libavcodec/vaapi_encode_h265.c | 24 ++++++++++-
>>>>    3 files changed, 93 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>>>> index 3bf379b1a0..bdba9726b2 100644
>>>> --- a/libavcodec/vaapi_encode.c
>>>> +++ b/libavcodec/vaapi_encode.c
>>>> @@ -848,9 +848,13 @@ static void
>>>> vaapi_encode_set_b_pictures(AVCodecContext
>>>> *avctx,
>>>>                pic->b_depth = current_depth;
>>>>
>>>>                vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
>>>> -            vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
>>>>                vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
>>>>
>>>> +            if (!ctx->b_to_gpb)
>>>> +                vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
>>>> +            else
>>>> +                vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
>>>
>>> This is doing something extremely dubious.  If b-to-gpb is set, then
>>> don't use the future reference?
>>
>> According to
>> https://github.com/intel/media-
>> driver/blob/master/media_driver/agnostic/common/codec/hal/codechal_vdenc
>> _hevc.cpp#L3072-L3087
>> , L0 and L1 should be the same for vdenc hevc on some platforms, so user can't
>> use past and future reference together, which is why you experienced the failure
>> after applying version 2
>>
>> Thanks
>> Haihao
>>
>>
>>> That means these pictures will only have the past reference, and the
>>> coding efficiency will often be much worse.
>>>
>>> E.g. if you have the default structure (max_b_frames = 2, max_b_depth
>>> = 1) then in a sequence of four pictures you get:
>>>
>>> 1 referring to something previous
>>> 4 referring to 1
>>> 2 referring to 1 and 4
>>> 3 referring to 1 and 4
>>>
>>> and this change means you lose the 2 -> 4 and 3 -> 4 references.
>>> Therefore, a change in the picture between 1 and 2 will end up coded
>>> three times in 2, 3 and 4 rather than just being coded in 4 and then referred to
>> by the others.
> 
> If driver doesn't support B frames with two different reference lists, use gpb instead
> of regular B is a best way. In V3, I turned B frames to P, but this will break gop structure.
> If user set I/P/B frames, while the output only contains I/P frames will be much confuse.

If the driver requires that RefPicList0 and RefPicList1 in B slices are the same then isn't the obvious solution to concatenate the expected lists and put the result in both actual lists (assuming it fits)?

That continues to support B frames in the obvious way (and transparently to the user).  The hardware will have to do a bit more motion search, but the fake P slices already require that anyway so I assume it is sensibly optimised.

>>>> +
>>>>                for (ref = end->refs[1]; ref; ref = ref->refs[1])
>>>>                    vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0);
>>>>            }
>>>> @@ -871,8 +875,11 @@ static void
>>>> vaapi_encode_set_b_pictures(AVCodecContext
>>>> *avctx,
>>>>
>>>>            vaapi_encode_add_ref(avctx, pic, pic,   0, 1, 0);
>>>>            vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
>>>> -        vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
>>>>            vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
>>>> +        if (!ctx->b_to_gpb)
>>>> +            vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
>>>> +        else
>>>> +            vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
>>>>
>>>>            for (ref = end->refs[1]; ref; ref = ref->refs[1])
>>>>                vaapi_encode_add_ref(avctx, pic, ref, 0, 1, 0); @@
>>>> -1845,6 +1852,51 @@ static av_cold int
>>>> vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>>>>            ref_l1 = attr.value >> 16 & 0xffff;
>>>>        }
>>>>
>>>> +    ctx->p_to_gpb = 0;
>>>> +    ctx->b_to_gpb = 0;
>>>> +
>>>> +#if VA_CHECK_VERSION(1, 9, 0)
>>>> +    if (!(ctx->codec->flags & FLAG_INTRA_ONLY ||
>>>> +        avctx->gop_size <= 1)) {
>>>> +        attr = (VAConfigAttrib) { VAConfigAttribPredictionDirection };
>>>> +        vas = vaGetConfigAttributes(ctx->hwctx->display,
>>>> +                                    ctx->va_profile,
>>>> +                                    ctx->va_entrypoint,
>>>> +                                    &attr, 1);
>>>> +        if (vas != VA_STATUS_SUCCESS) {
>>>> +            av_log(avctx, AV_LOG_WARNING, "Failed to query
>>>> +prediction
>>>> direction "
>>>> +                   "attribute: %d (%s).\n", vas, vaErrorStr(vas));
>>>> +            return AVERROR_EXTERNAL;
>>>> +        } else if (attr.value == VA_ATTRIB_NOT_SUPPORTED) {
>>>> +            av_log(avctx, AV_LOG_VERBOSE, "Driver does not report
>>>> + any
>>>> additional "
>>>> +                   "prediction constraints.\n");
>>>> +        } else {
>>>> +            if (((ref_l0 > 0 || ref_l1 > 0) && !(attr.value &
>>>> VA_PREDICTION_DIRECTION_PREVIOUS)) ||
>>>> +                ((ref_l1 == 0) && (attr.value &
>>>> (VA_PREDICTION_DIRECTION_FUTURE |
>>>> VA_PREDICTION_DIRECTION_BI_NOT_EMPTY)))) {
>>>> +                av_log(avctx, AV_LOG_ERROR, "Driver report
>>>> + incorrect
>>>> prediction "
>>>> +                       "direction attribute.\n");
>>>> +                return AVERROR_EXTERNAL;
>>>> +            }
>>>> +
>>>> +            if (!(attr.value & VA_PREDICTION_DIRECTION_FUTURE)) {
>>>> +                if (ref_l0 > 0 && ref_l1 > 0) {
>>>> +                    ctx->b_to_gpb = 1;
>>>> +                    av_log(avctx, AV_LOG_VERBOSE, "Driver support
>>>> + previous
>>>> prediction "
>>>> +                           "only for B-frames.\n");
>>>> +                }
>>>> +            }
>>>
>>> Please note that the PREDICTION_DIRECTION_FUTURE/PREVIOUS options
>>> don't mean anything for H.265.
>>>
>>> The driver isn't told which direction the prediction is in, it's only
>>> told about some reference frames and how to refer to them.  Whether
>>> those frames are in the past or future is unspecified and irrelevant -
>>> a P frame can refer to a single future frame if it wants.
>>>
>>> (I tried to argue this when it was added, but given that they are
>>> meaningless I didn't argue very hard.)
>>>
>>> I suspect you are trying to use this as a test of something else.
>>> Perhaps you could explain what the test you actually want is?
> 
> VA_PREDICTION_DIRECTION_PREVIOUS/ VA_PREDICTION_DIRECTION_FUTURE/ VA_PREDICTION_DIRECTION_BI_NOT_EMPTY
>   are used to indicate if driver has the limitation on how to set regular P and B frame's reference list when the queried max
> reference list ref_l0 and ref_l1 both are not zero.
> 
> If queried value is VA_PREDICTION_DIRECTION_PREVIOUS only, this means driver doesn't support B frame with different l0/l1,
> need to set l1 = 10.
> VA_PREDICTION_DIRECTION_PREVIOUS | VA_PREDICTION_DIRECTION_FUTURE means different l0/l1 is support for B frame.
> 
> And if queried value is VA_PREDICTION_DIRECTION_BI_NOT_EMPTY, this means driver doesn't support P frame with l0 only.

Hmm.  So if we ignore both the names and the documentation as unhelpful then the meanings you are intending are:

Attribute unset -> no restrictions.
Attribute set -> PREVIOUS bit must be set: P and B slices are both supported but RefPicList0 and RefPicList1 must in B slices must be equal.

Then additional bits modify that:

FUTURE bit set -> RefPicList0 and RefPicList1 in B slices may differ.
BI_NOT_EMPTY bit set -> P slices are not supported.

(If this is actually the intended meaning then maybe someone could add this to the documentation in libva?)

The real Intel hardware we've then got for H.265 is:

Gen9, Gen9 vdenc, Gen11, Gen12: unset -> no restrictions.
Gen11 vdenc: PREVIOUS | BI_NOT_EMPTY -> P slices are not supported, B slices must have RefPicList0 equal to RefPicList1.
Gen12 vdenc: PREVIOUS | FUTURE | BI_NOT_EMPTY -> P slices are not supported, B slices can do whatever.

For Gen12 vdenc you fake up P frames by using B slices with two equal lists, and make B frames normally.  That looks sensible.

For Gen11 vdenc you are faking up P frames in the same way, but then with B frames your patch pretends they are also P frames and ignores all but the first reference, which IMO is not sensible because it will multiply bitrate by several times at a given quality.  Hence suggestion above?

> And in debug message, maybe use "Driver only support same reference list for B-frames\n" will be more clear.

Yeah, given the trickiness of this it's probably a good idea if the messages are very clear about what is going on.

>>>> +
>>>> +            if (attr.value & VA_PREDICTION_DIRECTION_BI_NOT_EMPTY) {
>>>> +                if (ref_l0 > 0 && ref_l1 > 0) {
>>>> +                    ctx->p_to_gpb = 1;
>>>> +                    av_log(avctx, AV_LOG_VERBOSE, "Driver does not
>>>> + support
>>>> P-frames, "
>>>> +                           "replacing them with previous prediction
>>>> + only B-
>>>> frames.\n");
>>>> +                }
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +#endif
>>>> ...

- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Xiang, Haihao]

> 
> Hmm.  So if we ignore both the names and the documentation as unhelpful then
> the meanings you are intending are:
> 
> Attribute unset -> no restrictions.
> Attribute set -> PREVIOUS bit must be set: P and B slices are both supported
> but RefPicList0 and RefPicList1 must in B slices must be equal.
> 
> Then additional bits modify that:
> 
> FUTURE bit set -> RefPicList0 and RefPicList1 in B slices may differ.
> BI_NOT_EMPTY bit set -> P slices are not supported.
> 
> (If this is actually the intended meaning then maybe someone could add this to
> the documentation in libva?)

I submitted https://github.com/intel/libva/pull/581 to update the doc, you are
welcome to review it.

Thanks
Haihao

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi

[Mark Thompson]

On 14/03/2022 02:15, Xiang, Haihao wrote:
> On Sun, 2022-03-13 at 20:45 +0000, Mark Thompson wrote:
>> On 11/03/2022 09:00, Fei Wang wrote:
>>> From: Linjie Fu <linjie.fu@intel.com>
>>>
>>> Use GPB frames to replace regular P/B frames if backend driver does not
>>> support it.
>>>
>>> - GPB:
>>>       Generalized P and B picture. Regular P/B frames replaced by B
>>>       frames with previous-predict only, L0 == L1. Normal B frames
>>>       still have 2 different ref_lists and allow bi-prediction
>>>
>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
>>> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
>>> ---
>>> update:
>>> 1. Add b to gpb.
>>> 2. Optimise debug message.
>>>
>>>    libavcodec/vaapi_encode.c      | 74 +++++++++++++++++++++++++++++++---
>>>    libavcodec/vaapi_encode.h      |  2 +
>>>    libavcodec/vaapi_encode_h265.c | 24 ++++++++++-
>>>    3 files changed, 93 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>>> index 3bf379b1a0..bdba9726b2 100644
>>> --- a/libavcodec/vaapi_encode.c
>>> +++ b/libavcodec/vaapi_encode.c
>>> @@ -848,9 +848,13 @@ static void vaapi_encode_set_b_pictures(AVCodecContext
>>> *avctx,
>>>                pic->b_depth = current_depth;
>>>    
>>>                vaapi_encode_add_ref(avctx, pic, start, 1, 1, 0);
>>> -            vaapi_encode_add_ref(avctx, pic, end,   1, 1, 0);
>>>                vaapi_encode_add_ref(avctx, pic, prev,  0, 0, 1);
>>>    
>>> +            if (!ctx->b_to_gpb)
>>> +                vaapi_encode_add_ref(avctx, pic, end, 1, 1, 0);
>>> +            else
>>> +                vaapi_encode_add_ref(avctx, pic, end, 0, 1, 0);
>>
>> This is doing something extremely dubious.  If b-to-gpb is set, then don't use
>> the future reference?
> 
> According to
> https://github.com/intel/media-driver/blob/master/media_driver/agnostic/common/codec/hal/codechal_vdenc_hevc.cpp#L3072-L3087
> , L0 and L1 should be the same for vdenc hevc on some platforms,

Right, so this is actually a different constraint which isn't indicated so far.  It's not just the BI_NOT_EMPTY constraint that P slices cannot be used, it's also that in B slices RefPicList1 has to be identical to RefPicList0.  Perhaps this should be added to libva?

>                                                                  so user can't
> use past and future reference together,

Where is this coming from?  I don't see how past vs. future references are relevant at all (since the driver can't see that anyway), only the matching content of the lists.

Maybe in this case the right behaviour would be to concatenate what would normally be in the two lists and then put that in both of them (assuming it supports the necessary reference counts, but this particular hardware appears to allow 3/3 so it would).

>                                         which is why you experienced the failure
> after applying version 2

Yep - and is also why the P-frame only case works, since that puts the same single entry in both lists.

- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".