[FFmpeg-devel] [PATCH v4 1/4] lavc/vaapi_encode_h265: Add GPB frame support for hevc_vaapi
Wang, Fei W
fei.w.wang at intel.com
Mon Mar 14 13:07:46 EET 2022
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Xiang,
> Haihao
> Sent: Monday, March 14, 2022 10:15 AM
> To: ffmpeg-devel at 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 at 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 at intel.com>
> > > Signed-off-by: Fei Wang <fei.w.wang at 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 at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
> with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list