| Message ID | 20190313001748.11781-5-sw@jkqxz.net |
|---|---|
| State | New |
| Headers | show |
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Wednesday, March 13, 2019 8:18 AM > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support > > --- > libavcodec/vaapi_encode.c | 128 > ++++++++++++++++++++++++++++++++ > libavcodec/vaapi_encode.h | 11 +++ > libavcodec/vaapi_encode_h264.c | 2 + > libavcodec/vaapi_encode_h265.c | 2 + > libavcodec/vaapi_encode_mpeg2.c | 2 + > libavcodec/vaapi_encode_vp8.c | 2 + > libavcodec/vaapi_encode_vp9.c | 2 + > 7 files changed, 149 insertions(+) > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 2dda451882..03a7f5ce3e 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext > *avctx, > int err, i; > char data[MAX_PARAM_BUFFER_SIZE]; > size_t bit_len; > + AVFrameSideData *sd; > > av_log(avctx, AV_LOG_DEBUG, "Issuing encode for > pic %"PRId64"/%"PRId64" " > "as type %s.\n", pic->display_order, pic->encode_order, > @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext > *avctx, > } > } > > + sd = av_frame_get_side_data(pic->input_image, > + AV_FRAME_DATA_REGIONS_OF_INTEREST); > + > +#if VA_CHECK_VERSION(1, 0, 0) > + if (sd && ctx->roi_allowed) { > + const AVRegionOfInterest *roi; > + int nb_roi; > + VAEncMiscParameterBuffer param_misc = { > + .type = VAEncMiscParameterTypeROI, > + }; > + VAEncMiscParameterBufferROI param_roi; > + // VAAPI requires the second structure to immediately follow the > + // first in the parameter buffer, but they have different natural > + // alignment on 64-bit architectures (4 vs. 8, since the ROI > + // structure contains a pointer). This means we can't make a > + // single working structure, nor can we make the buffer > + // separately and map it because dereferencing the second pointer > + // would be undefined. Therefore, construct the two parts > + // separately and combine them into a single character buffer > + // before uploading. > + char param_buffer[sizeof(param_misc) + sizeof(param_roi)]; > + int i, v; > + > + roi = (const AVRegionOfInterest*)sd->data; > + av_assert0(roi->self_size && sd->size % roi->self_size == 0); it is possible if the user forget to set the value of roi->self_size. assert is not feasible. > + nb_roi = sd->size / roi->self_size; > + if (nb_roi > ctx->roi_regions) { > + if (!ctx->roi_warned) { > + av_log(avctx, AV_LOG_WARNING, "More ROIs set than " > + "supported by driver (%d > %d).\n", > + nb_roi, ctx->roi_regions); > + ctx->roi_warned = 1; > + } > + nb_roi = ctx->roi_regions; > + } > + > + pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi)); > + if (!pic->roi) { > + err = AVERROR(ENOMEM); > + goto fail; > + } shall we add comments here to explain the list visit order? > + for (i = 0; i < nb_roi; i++) { > + roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i); same comment as libx264 > + > + v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den; shall we check here if roi->qoffset-den is zero? > + av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) - > > %+d.\n", > + roi->x, roi->y, roi->width, roi->height, v); > + > + pic->roi[i] = (VAEncROI) { > + .roi_rectangle = { > + .x = roi->x, > + .y = roi->y, > + .width = roi->width, > + .height = roi->height, > + }, > + .roi_value = av_clip_c(v, INT8_MIN, INT8_MAX), > + }; > + } > + > + param_roi = (VAEncMiscParameterBufferROI) { > + .num_roi = nb_roi, > + .max_delta_qp = INT8_MAX, > + .min_delta_qp = 0, > + .roi = pic->roi, > + .roi_flags.bits.roi_value_is_qp_delta = 1, > + }; > + > + memcpy(param_buffer, ¶m_misc, sizeof(param_misc)); > + memcpy(param_buffer + sizeof(param_misc), > + ¶m_roi, sizeof(param_roi)); > + > + err = vaapi_encode_make_param_buffer(avctx, pic, > + VAEncMiscParameterBufferType, > + param_buffer, > + sizeof(param_buffer)); > + if (err < 0) > + goto fail; > + } else > +#endif > + if (sd && !ctx->roi_warned) { > + av_log(avctx, AV_LOG_WARNING, "ROI side data on input " > + "frames ignored due to lack of driver support.\n"); > + ctx->roi_warned = 1; > + } > + > vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context, > pic->input_surface); > if (vas != VA_STATUS_SUCCESS) { > @@ -477,6 +563,7 @@ fail_at_end: > av_freep(&pic->codec_picture_params); > av_freep(&pic->param_buffers); > av_freep(&pic->slices); > + av_freep(&pic->roi); > av_frame_free(&pic->recon_image); > av_buffer_unref(&pic->output_buffer_ref); > pic->output_buffer = VA_INVALID_ID; > @@ -611,6 +698,7 @@ static int vaapi_encode_free(AVCodecContext *avctx, > > av_freep(&pic->priv_data); > av_freep(&pic->codec_picture_params); > + av_freep(&pic->roi); it is unnecessary since it is already freed in function vaapi_encode_issue. > > av_free(pic); > > @@ -1899,6 +1987,42 @@ static av_cold int > vaapi_encode_init_quality(AVCodecContext *avctx) > return 0; > } > > +static av_cold int vaapi_encode_init_roi(AVCodecContext *avctx) > +{ > + VAAPIEncodeContext *ctx = avctx->priv_data; > + > +#if VA_CHECK_VERSION(1, 0, 0) > + VAStatus vas; > + VAConfigAttrib attr = { VAConfigAttribEncROI }; > + > + 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 ROI " > + "config attribute: %d (%s).\n", vas, vaErrorStr(vas)); > + return AVERROR_EXTERNAL; > + } > + > + if (attr.value == VA_ATTRIB_NOT_SUPPORTED) { > + ctx->roi_allowed = 0; > + } else { > + VAConfigAttribValEncROI roi = { > + .value = attr.value, > + }; > + > + ctx->roi_regions = roi.bits.num_roi_regions; > + ctx->roi_allowed = ctx->roi_regions > 0 && > + (ctx->va_rc_mode == VA_RC_CQP || > + roi.bits.roi_rc_qp_delta_support); > + } > +#else > + ctx->roi_allowed = 0; it is unnecessary since it is never checked out of the #if body > +#endif > + return 0; > +} > + > static void vaapi_encode_free_output_buffer(void *opaque, > uint8_t *data) > { > @@ -2089,6 +2213,10 @@ av_cold int > ff_vaapi_encode_init(AVCodecContext *avctx) > if (err < 0) > goto fail; > > + err = vaapi_encode_init_roi(avctx); > + if (err < 0) > + goto fail; > + > if (avctx->compression_level >= 0) { > err = vaapi_encode_init_quality(avctx); > if (err < 0) > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h > index 44a8db566e..ee074b4dc1 100644 > --- a/libavcodec/vaapi_encode.h > +++ b/libavcodec/vaapi_encode.h > @@ -69,6 +69,11 @@ typedef struct VAAPIEncodePicture { > int64_t pts; > int force_idr; > > +#if VA_CHECK_VERSION(1, 0, 0) > + // ROI regions. > + VAEncROI *roi; > +#endif > + > int type; > int b_depth; > int encode_issued; > @@ -314,6 +319,12 @@ typedef struct VAAPIEncodeContext { > int idr_counter; > int gop_counter; > int end_of_stream; > + > + // ROI state. > + int roi_allowed; > + int roi_regions; it might be more straight forward if rename it to roi_max_regions. anyway, both are ok. > + int roi_quant_range; > + int roi_warned; > } VAAPIEncodeContext; > > enum { > diff --git a/libavcodec/vaapi_encode_h264.c > b/libavcodec/vaapi_encode_h264.c > index 91be33f99f..7c55b8a4a7 100644 > --- a/libavcodec/vaapi_encode_h264.c > +++ b/libavcodec/vaapi_encode_h264.c > @@ -1123,6 +1123,8 @@ static av_cold int > vaapi_encode_h264_configure(AVCodecContext *avctx) > } > } > > + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); > + > return 0; > } > > diff --git a/libavcodec/vaapi_encode_h265.c > b/libavcodec/vaapi_encode_h265.c > index 758bd40a37..538862a9d5 100644 > --- a/libavcodec/vaapi_encode_h265.c > +++ b/libavcodec/vaapi_encode_h265.c > @@ -1102,6 +1102,8 @@ static av_cold int > vaapi_encode_h265_configure(AVCodecContext *avctx) > priv->fixed_qp_b = 30; > } > > + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); > + > return 0; > } > > diff --git a/libavcodec/vaapi_encode_mpeg2.c > b/libavcodec/vaapi_encode_mpeg2.c > index fb1ef71fdc..bac9ea1fa6 100644 > --- a/libavcodec/vaapi_encode_mpeg2.c > +++ b/libavcodec/vaapi_encode_mpeg2.c > @@ -552,6 +552,8 @@ static av_cold int > vaapi_encode_mpeg2_configure(AVCodecContext *avctx) > ctx->nb_slices = ctx->slice_block_rows; > ctx->slice_size = 1; > > + ctx->roi_quant_range = 31; > + > return 0; > } > > diff --git a/libavcodec/vaapi_encode_vp8.c > b/libavcodec/vaapi_encode_vp8.c > index ddbe4c9075..6e7bf9d106 100644 > --- a/libavcodec/vaapi_encode_vp8.c > +++ b/libavcodec/vaapi_encode_vp8.c > @@ -173,6 +173,8 @@ static av_cold int > vaapi_encode_vp8_configure(AVCodecContext *avctx) > else > priv->q_index_i = priv->q_index_p; > > + ctx->roi_quant_range = VP8_MAX_QUANT; > + > return 0; > } > > diff --git a/libavcodec/vaapi_encode_vp9.c > b/libavcodec/vaapi_encode_vp9.c > index f89fd0d07a..d7f415d704 100644 > --- a/libavcodec/vaapi_encode_vp9.c > +++ b/libavcodec/vaapi_encode_vp9.c > @@ -202,6 +202,8 @@ static av_cold int > vaapi_encode_vp9_configure(AVCodecContext *avctx) > priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100; > } > > + ctx->roi_quant_range = VP9_MAX_QUANT; > + > return 0; > } > > -- > 2.19.2 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 13/03/2019 02:46, Guo, Yejun wrote: >> -----Original Message----- >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Mark Thompson >> Sent: Wednesday, March 13, 2019 8:18 AM >> To: ffmpeg-devel@ffmpeg.org >> Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support >> >> --- >> libavcodec/vaapi_encode.c | 128 >> ++++++++++++++++++++++++++++++++ >> libavcodec/vaapi_encode.h | 11 +++ >> libavcodec/vaapi_encode_h264.c | 2 + >> libavcodec/vaapi_encode_h265.c | 2 + >> libavcodec/vaapi_encode_mpeg2.c | 2 + >> libavcodec/vaapi_encode_vp8.c | 2 + >> libavcodec/vaapi_encode_vp9.c | 2 + >> 7 files changed, 149 insertions(+) >> >> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c >> index 2dda451882..03a7f5ce3e 100644 >> --- a/libavcodec/vaapi_encode.c >> +++ b/libavcodec/vaapi_encode.c >> @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext >> *avctx, >> int err, i; >> char data[MAX_PARAM_BUFFER_SIZE]; >> size_t bit_len; >> + AVFrameSideData *sd; >> >> av_log(avctx, AV_LOG_DEBUG, "Issuing encode for >> pic %"PRId64"/%"PRId64" " >> "as type %s.\n", pic->display_order, pic->encode_order, >> @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext >> *avctx, >> } >> } >> >> + sd = av_frame_get_side_data(pic->input_image, >> + AV_FRAME_DATA_REGIONS_OF_INTEREST); >> + >> +#if VA_CHECK_VERSION(1, 0, 0) >> + if (sd && ctx->roi_allowed) { >> + const AVRegionOfInterest *roi; >> + int nb_roi; >> + VAEncMiscParameterBuffer param_misc = { >> + .type = VAEncMiscParameterTypeROI, >> + }; >> + VAEncMiscParameterBufferROI param_roi; >> + // VAAPI requires the second structure to immediately follow the >> + // first in the parameter buffer, but they have different natural >> + // alignment on 64-bit architectures (4 vs. 8, since the ROI >> + // structure contains a pointer). This means we can't make a >> + // single working structure, nor can we make the buffer >> + // separately and map it because dereferencing the second pointer >> + // would be undefined. Therefore, construct the two parts >> + // separately and combine them into a single character buffer >> + // before uploading. >> + char param_buffer[sizeof(param_misc) + sizeof(param_roi)]; >> + int i, v; >> + >> + roi = (const AVRegionOfInterest*)sd->data; >> + av_assert0(roi->self_size && sd->size % roi->self_size == 0); > > it is possible if the user forget to set the value of roi->self_size. > assert is not feasible. Not setting self_size violates the API contract ("Must be set to the size of this data structure") and therefore invokes undefined behaviour. Aborting when undefined behaviour is first detected is the correct response. >> + nb_roi = sd->size / roi->self_size; >> + if (nb_roi > ctx->roi_regions) { >> + if (!ctx->roi_warned) { >> + av_log(avctx, AV_LOG_WARNING, "More ROIs set than " >> + "supported by driver (%d > %d).\n", >> + nb_roi, ctx->roi_regions); >> + ctx->roi_warned = 1; >> + } >> + nb_roi = ctx->roi_regions; >> + } >> + >> + pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi)); >> + if (!pic->roi) { >> + err = AVERROR(ENOMEM); >> + goto fail; >> + } > > shall we add comments here to explain the list visit order? Sure, added. >> + for (i = 0; i < nb_roi; i++) { >> + roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i); > > same comment as libx264 I guess, though I'm not sure filling the code with guards detecting undefined behaviour cases really has any value. I've added an additional note on the API that the value must be the same on all entries. >> + >> + v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den; > > shall we check here if roi->qoffset-den is zero? Sure, I'll add an assert for the invalid fraction since the API requires [-1,+1]. >> + av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) - >>> %+d.\n", >> + roi->x, roi->y, roi->width, roi->height, v); >> + >> + pic->roi[i] = (VAEncROI) { >> + .roi_rectangle = { >> + .x = roi->x, >> + .y = roi->y, >> + .width = roi->width, >> + .height = roi->height, >> + }, >> + .roi_value = av_clip_c(v, INT8_MIN, INT8_MAX), >> + }; >> + } >> + >> + param_roi = (VAEncMiscParameterBufferROI) { >> + .num_roi = nb_roi, >> + .max_delta_qp = INT8_MAX, >> + .min_delta_qp = 0, >> + .roi = pic->roi, >> + .roi_flags.bits.roi_value_is_qp_delta = 1, >> + }; >> + >> + memcpy(param_buffer, ¶m_misc, sizeof(param_misc)); >> + memcpy(param_buffer + sizeof(param_misc), >> + ¶m_roi, sizeof(param_roi)); >> + >> + err = vaapi_encode_make_param_buffer(avctx, pic, >> + VAEncMiscParameterBufferType, >> + param_buffer, >> + sizeof(param_buffer)); >> + if (err < 0) >> + goto fail; >> + } else >> +#endif >> + if (sd && !ctx->roi_warned) { >> + av_log(avctx, AV_LOG_WARNING, "ROI side data on input " >> + "frames ignored due to lack of driver support.\n"); >> + ctx->roi_warned = 1; >> + } >> + >> vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context, >> pic->input_surface); >> if (vas != VA_STATUS_SUCCESS) { >> @@ -477,6 +563,7 @@ fail_at_end: >> av_freep(&pic->codec_picture_params); >> av_freep(&pic->param_buffers); >> av_freep(&pic->slices); >> + av_freep(&pic->roi); >> av_frame_free(&pic->recon_image); >> av_buffer_unref(&pic->output_buffer_ref); >> pic->output_buffer = VA_INVALID_ID; >> @@ -611,6 +698,7 @@ static int vaapi_encode_free(AVCodecContext *avctx, >> >> av_freep(&pic->priv_data); >> av_freep(&pic->codec_picture_params); >> + av_freep(&pic->roi); > > it is unnecessary since it is already freed in function vaapi_encode_issue. Not if the issue succeeds. (Indeed, it can't be in that case because the encode might be asynchronous.) >> >> av_free(pic); >> >> @@ -1899,6 +1987,42 @@ static av_cold int >> vaapi_encode_init_quality(AVCodecContext *avctx) >> return 0; >> } >> >> +static av_cold int vaapi_encode_init_roi(AVCodecContext *avctx) >> +{ >> + VAAPIEncodeContext *ctx = avctx->priv_data; >> + >> +#if VA_CHECK_VERSION(1, 0, 0) >> + VAStatus vas; >> + VAConfigAttrib attr = { VAConfigAttribEncROI }; >> + >> + 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 ROI " >> + "config attribute: %d (%s).\n", vas, vaErrorStr(vas)); >> + return AVERROR_EXTERNAL; >> + } >> + >> + if (attr.value == VA_ATTRIB_NOT_SUPPORTED) { >> + ctx->roi_allowed = 0; >> + } else { >> + VAConfigAttribValEncROI roi = { >> + .value = attr.value, >> + }; >> + >> + ctx->roi_regions = roi.bits.num_roi_regions; >> + ctx->roi_allowed = ctx->roi_regions > 0 && >> + (ctx->va_rc_mode == VA_RC_CQP || >> + roi.bits.roi_rc_qp_delta_support); >> + } >> +#else >> + ctx->roi_allowed = 0; > > it is unnecessary since it is never checked out of the #if body I guess, removed. >> +#endif >> + return 0; >> +} >> + >> static void vaapi_encode_free_output_buffer(void *opaque, >> uint8_t *data) >> { >> @@ -2089,6 +2213,10 @@ av_cold int >> ff_vaapi_encode_init(AVCodecContext *avctx) >> if (err < 0) >> goto fail; >> >> + err = vaapi_encode_init_roi(avctx); >> + if (err < 0) >> + goto fail; >> + >> if (avctx->compression_level >= 0) { >> err = vaapi_encode_init_quality(avctx); >> if (err < 0) >> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h >> index 44a8db566e..ee074b4dc1 100644 >> --- a/libavcodec/vaapi_encode.h >> +++ b/libavcodec/vaapi_encode.h >> @@ -69,6 +69,11 @@ typedef struct VAAPIEncodePicture { >> int64_t pts; >> int force_idr; >> >> +#if VA_CHECK_VERSION(1, 0, 0) >> + // ROI regions. >> + VAEncROI *roi; >> +#endif >> + >> int type; >> int b_depth; >> int encode_issued; >> @@ -314,6 +319,12 @@ typedef struct VAAPIEncodeContext { >> int idr_counter; >> int gop_counter; >> int end_of_stream; >> + >> + // ROI state. >> + int roi_allowed; >> + int roi_regions; > > it might be more straight forward if rename it to roi_max_regions. > anyway, both are ok. Yes, I agree; changed. >> + int roi_quant_range; >> + int roi_warned; >> } VAAPIEncodeContext; >> >> enum { >> diff --git a/libavcodec/vaapi_encode_h264.c >> b/libavcodec/vaapi_encode_h264.c >> index 91be33f99f..7c55b8a4a7 100644 >> --- a/libavcodec/vaapi_encode_h264.c >> +++ b/libavcodec/vaapi_encode_h264.c >> @@ -1123,6 +1123,8 @@ static av_cold int >> vaapi_encode_h264_configure(AVCodecContext *avctx) >> } >> } >> >> + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); >> + >> return 0; >> } >> >> diff --git a/libavcodec/vaapi_encode_h265.c >> b/libavcodec/vaapi_encode_h265.c >> index 758bd40a37..538862a9d5 100644 >> --- a/libavcodec/vaapi_encode_h265.c >> +++ b/libavcodec/vaapi_encode_h265.c >> @@ -1102,6 +1102,8 @@ static av_cold int >> vaapi_encode_h265_configure(AVCodecContext *avctx) >> priv->fixed_qp_b = 30; >> } >> >> + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); >> + >> return 0; >> } >> >> diff --git a/libavcodec/vaapi_encode_mpeg2.c >> b/libavcodec/vaapi_encode_mpeg2.c >> index fb1ef71fdc..bac9ea1fa6 100644 >> --- a/libavcodec/vaapi_encode_mpeg2.c >> +++ b/libavcodec/vaapi_encode_mpeg2.c >> @@ -552,6 +552,8 @@ static av_cold int >> vaapi_encode_mpeg2_configure(AVCodecContext *avctx) >> ctx->nb_slices = ctx->slice_block_rows; >> ctx->slice_size = 1; >> >> + ctx->roi_quant_range = 31; >> + >> return 0; >> } >> >> diff --git a/libavcodec/vaapi_encode_vp8.c >> b/libavcodec/vaapi_encode_vp8.c >> index ddbe4c9075..6e7bf9d106 100644 >> --- a/libavcodec/vaapi_encode_vp8.c >> +++ b/libavcodec/vaapi_encode_vp8.c >> @@ -173,6 +173,8 @@ static av_cold int >> vaapi_encode_vp8_configure(AVCodecContext *avctx) >> else >> priv->q_index_i = priv->q_index_p; >> >> + ctx->roi_quant_range = VP8_MAX_QUANT; >> + >> return 0; >> } >> >> diff --git a/libavcodec/vaapi_encode_vp9.c >> b/libavcodec/vaapi_encode_vp9.c >> index f89fd0d07a..d7f415d704 100644 >> --- a/libavcodec/vaapi_encode_vp9.c >> +++ b/libavcodec/vaapi_encode_vp9.c >> @@ -202,6 +202,8 @@ static av_cold int >> vaapi_encode_vp9_configure(AVCodecContext *avctx) >> priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100; >> } >> >> + ctx->roi_quant_range = VP9_MAX_QUANT; >> + >> return 0; >> } >> >> -- >> 2.19.2 >> Thanks, - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Wednesday, March 13, 2019 5:44 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support > > On 13/03/2019 02:46, Guo, Yejun wrote: > >> -----Original Message----- > >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > >> Of Mark Thompson > >> Sent: Wednesday, March 13, 2019 8:18 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support > >> > >> --- > >> libavcodec/vaapi_encode.c | 128 > >> ++++++++++++++++++++++++++++++++ > >> libavcodec/vaapi_encode.h | 11 +++ > >> libavcodec/vaapi_encode_h264.c | 2 + > >> libavcodec/vaapi_encode_h265.c | 2 + > >> libavcodec/vaapi_encode_mpeg2.c | 2 + > >> libavcodec/vaapi_encode_vp8.c | 2 + > >> libavcodec/vaapi_encode_vp9.c | 2 + > >> 7 files changed, 149 insertions(+) > >> > >> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > >> index 2dda451882..03a7f5ce3e 100644 > >> --- a/libavcodec/vaapi_encode.c > >> +++ b/libavcodec/vaapi_encode.c > >> @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext > >> *avctx, > >> int err, i; > >> char data[MAX_PARAM_BUFFER_SIZE]; > >> size_t bit_len; > >> + AVFrameSideData *sd; > >> > >> av_log(avctx, AV_LOG_DEBUG, "Issuing encode for > >> pic %"PRId64"/%"PRId64" " > >> "as type %s.\n", pic->display_order, pic->encode_order, > >> @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext > >> *avctx, > >> } > >> } > >> > >> + sd = av_frame_get_side_data(pic->input_image, > >> + AV_FRAME_DATA_REGIONS_OF_INTEREST); > >> + > >> +#if VA_CHECK_VERSION(1, 0, 0) > >> + if (sd && ctx->roi_allowed) { > >> + const AVRegionOfInterest *roi; > >> + int nb_roi; > >> + VAEncMiscParameterBuffer param_misc = { > >> + .type = VAEncMiscParameterTypeROI, > >> + }; > >> + VAEncMiscParameterBufferROI param_roi; > >> + // VAAPI requires the second structure to immediately follow the > >> + // first in the parameter buffer, but they have different natural > >> + // alignment on 64-bit architectures (4 vs. 8, since the ROI > >> + // structure contains a pointer). This means we can't make a > >> + // single working structure, nor can we make the buffer > >> + // separately and map it because dereferencing the second pointer > >> + // would be undefined. Therefore, construct the two parts > >> + // separately and combine them into a single character buffer > >> + // before uploading. > >> + char param_buffer[sizeof(param_misc) + sizeof(param_roi)]; > >> + int i, v; > >> + > >> + roi = (const AVRegionOfInterest*)sd->data; > >> + av_assert0(roi->self_size && sd->size % roi->self_size == 0); > > > > it is possible if the user forget to set the value of roi->self_size. > > assert is not feasible. > > Not setting self_size violates the API contract ("Must be set to the size of this > data structure") and therefore invokes undefined behaviour. Aborting when > undefined behaviour is first detected is the correct response. > > >> + nb_roi = sd->size / roi->self_size; > >> + if (nb_roi > ctx->roi_regions) { > >> + if (!ctx->roi_warned) { > >> + av_log(avctx, AV_LOG_WARNING, "More ROIs set than " > >> + "supported by driver (%d > %d).\n", > >> + nb_roi, ctx->roi_regions); > >> + ctx->roi_warned = 1; > >> + } > >> + nb_roi = ctx->roi_regions; > >> + } > >> + > >> + pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi)); > >> + if (!pic->roi) { > >> + err = AVERROR(ENOMEM); > >> + goto fail; > >> + } > > > > shall we add comments here to explain the list visit order? > > Sure, added. > > >> + for (i = 0; i < nb_roi; i++) { > >> + roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i); > > > > same comment as libx264 > > I guess, though I'm not sure filling the code with guards detecting undefined > behaviour cases really has any value. > > I've added an additional note on the API that the value must be the same on > all entries. I personally prefer code check and report error, anyway, I do not insist it. > > >> + > >> + v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den; > > > > shall we check here if roi->qoffset-den is zero? > > Sure, I'll add an assert for the invalid fraction since the API requires [-1,+1]. > > >> + av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) - > >>> %+d.\n", > >> + roi->x, roi->y, roi->width, roi->height, v); > >> + > >> + pic->roi[i] = (VAEncROI) { > >> + .roi_rectangle = { > >> + .x = roi->x, > >> + .y = roi->y, > >> + .width = roi->width, > >> + .height = roi->height, > >> + }, > >> + .roi_value = av_clip_c(v, INT8_MIN, INT8_MAX), > >> + }; > >> + } > >> + > >> + param_roi = (VAEncMiscParameterBufferROI) { > >> + .num_roi = nb_roi, > >> + .max_delta_qp = INT8_MAX, > >> + .min_delta_qp = 0, > >> + .roi = pic->roi, > >> + .roi_flags.bits.roi_value_is_qp_delta = 1, > >> + }; > >> + > >> + memcpy(param_buffer, ¶m_misc, sizeof(param_misc)); > >> + memcpy(param_buffer + sizeof(param_misc), > >> + ¶m_roi, sizeof(param_roi)); > >> + > >> + err = vaapi_encode_make_param_buffer(avctx, pic, > >> + VAEncMiscParameterBufferType, > >> + param_buffer, > >> + sizeof(param_buffer)); > >> + if (err < 0) > >> + goto fail; > >> + } else > >> +#endif > >> + if (sd && !ctx->roi_warned) { > >> + av_log(avctx, AV_LOG_WARNING, "ROI side data on input " > >> + "frames ignored due to lack of driver support.\n"); > >> + ctx->roi_warned = 1; > >> + } > >> + > >> vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context, > >> pic->input_surface); > >> if (vas != VA_STATUS_SUCCESS) { > >> @@ -477,6 +563,7 @@ fail_at_end: > >> av_freep(&pic->codec_picture_params); > >> av_freep(&pic->param_buffers); > >> av_freep(&pic->slices); > >> + av_freep(&pic->roi); > >> av_frame_free(&pic->recon_image); > >> av_buffer_unref(&pic->output_buffer_ref); > >> pic->output_buffer = VA_INVALID_ID; > >> @@ -611,6 +698,7 @@ static int vaapi_encode_free(AVCodecContext > *avctx, > >> > >> av_freep(&pic->priv_data); > >> av_freep(&pic->codec_picture_params); > >> + av_freep(&pic->roi); > > > > it is unnecessary since it is already freed in function vaapi_encode_issue. > > Not if the issue succeeds. (Indeed, it can't be in that case because the > encode might be asynchronous.) if the encoder is asynchronous, the av_freep(&pic->roi) in function vaapi_encode_issue is wrong, it might free the memory too early. I don't see a mechanism in av_freep that will wait for the asynchronous model, it just free the memory immediately. we can just remove av_freep(&pic->roi) in function vaapi_encode_issue. > > >> > >> av_free(pic); > >> > >> @@ -1899,6 +1987,42 @@ static av_cold int > >> vaapi_encode_init_quality(AVCodecContext *avctx) > >> return 0; > >> } > >> > >> +static av_cold int vaapi_encode_init_roi(AVCodecContext *avctx) > >> +{ > >> + VAAPIEncodeContext *ctx = avctx->priv_data; > >> + > >> +#if VA_CHECK_VERSION(1, 0, 0) > >> + VAStatus vas; > >> + VAConfigAttrib attr = { VAConfigAttribEncROI }; > >> + > >> + 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 ROI " > >> + "config attribute: %d (%s).\n", vas, vaErrorStr(vas)); > >> + return AVERROR_EXTERNAL; > >> + } > >> + > >> + if (attr.value == VA_ATTRIB_NOT_SUPPORTED) { > >> + ctx->roi_allowed = 0; > >> + } else { > >> + VAConfigAttribValEncROI roi = { > >> + .value = attr.value, > >> + }; > >> + > >> + ctx->roi_regions = roi.bits.num_roi_regions; > >> + ctx->roi_allowed = ctx->roi_regions > 0 && > >> + (ctx->va_rc_mode == VA_RC_CQP || > >> + roi.bits.roi_rc_qp_delta_support); > >> + } > >> +#else > >> + ctx->roi_allowed = 0; > > > > it is unnecessary since it is never checked out of the #if body > > I guess, removed. > > >> +#endif > >> + return 0; > >> +} > >> + > >> static void vaapi_encode_free_output_buffer(void *opaque, > >> uint8_t *data) > >> { > >> @@ -2089,6 +2213,10 @@ av_cold int > >> ff_vaapi_encode_init(AVCodecContext *avctx) > >> if (err < 0) > >> goto fail; > >> > >> + err = vaapi_encode_init_roi(avctx); > >> + if (err < 0) > >> + goto fail; > >> + > >> if (avctx->compression_level >= 0) { > >> err = vaapi_encode_init_quality(avctx); > >> if (err < 0) > >> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h > >> index 44a8db566e..ee074b4dc1 100644 > >> --- a/libavcodec/vaapi_encode.h > >> +++ b/libavcodec/vaapi_encode.h > >> @@ -69,6 +69,11 @@ typedef struct VAAPIEncodePicture { > >> int64_t pts; > >> int force_idr; > >> > >> +#if VA_CHECK_VERSION(1, 0, 0) > >> + // ROI regions. > >> + VAEncROI *roi; > >> +#endif > >> + > >> int type; > >> int b_depth; > >> int encode_issued; > >> @@ -314,6 +319,12 @@ typedef struct VAAPIEncodeContext { > >> int idr_counter; > >> int gop_counter; > >> int end_of_stream; > >> + > >> + // ROI state. > >> + int roi_allowed; > >> + int roi_regions; > > > > it might be more straight forward if rename it to roi_max_regions. > > anyway, both are ok. > > Yes, I agree; changed. > > >> + int roi_quant_range; > >> + int roi_warned; > >> } VAAPIEncodeContext; > >> > >> enum { > >> diff --git a/libavcodec/vaapi_encode_h264.c > >> b/libavcodec/vaapi_encode_h264.c > >> index 91be33f99f..7c55b8a4a7 100644 > >> --- a/libavcodec/vaapi_encode_h264.c > >> +++ b/libavcodec/vaapi_encode_h264.c > >> @@ -1123,6 +1123,8 @@ static av_cold int > >> vaapi_encode_h264_configure(AVCodecContext *avctx) > >> } > >> } > >> > >> + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); > >> + > >> return 0; > >> } > >> > >> diff --git a/libavcodec/vaapi_encode_h265.c > >> b/libavcodec/vaapi_encode_h265.c > >> index 758bd40a37..538862a9d5 100644 > >> --- a/libavcodec/vaapi_encode_h265.c > >> +++ b/libavcodec/vaapi_encode_h265.c > >> @@ -1102,6 +1102,8 @@ static av_cold int > >> vaapi_encode_h265_configure(AVCodecContext *avctx) > >> priv->fixed_qp_b = 30; > >> } > >> > >> + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); > >> + > >> return 0; > >> } > >> > >> diff --git a/libavcodec/vaapi_encode_mpeg2.c > >> b/libavcodec/vaapi_encode_mpeg2.c > >> index fb1ef71fdc..bac9ea1fa6 100644 > >> --- a/libavcodec/vaapi_encode_mpeg2.c > >> +++ b/libavcodec/vaapi_encode_mpeg2.c > >> @@ -552,6 +552,8 @@ static av_cold int > >> vaapi_encode_mpeg2_configure(AVCodecContext *avctx) > >> ctx->nb_slices = ctx->slice_block_rows; > >> ctx->slice_size = 1; > >> > >> + ctx->roi_quant_range = 31; > >> + > >> return 0; > >> } > >> > >> diff --git a/libavcodec/vaapi_encode_vp8.c > >> b/libavcodec/vaapi_encode_vp8.c > >> index ddbe4c9075..6e7bf9d106 100644 > >> --- a/libavcodec/vaapi_encode_vp8.c > >> +++ b/libavcodec/vaapi_encode_vp8.c > >> @@ -173,6 +173,8 @@ static av_cold int > >> vaapi_encode_vp8_configure(AVCodecContext *avctx) > >> else > >> priv->q_index_i = priv->q_index_p; > >> > >> + ctx->roi_quant_range = VP8_MAX_QUANT; > >> + > >> return 0; > >> } > >> > >> diff --git a/libavcodec/vaapi_encode_vp9.c > >> b/libavcodec/vaapi_encode_vp9.c > >> index f89fd0d07a..d7f415d704 100644 > >> --- a/libavcodec/vaapi_encode_vp9.c > >> +++ b/libavcodec/vaapi_encode_vp9.c > >> @@ -202,6 +202,8 @@ static av_cold int > >> vaapi_encode_vp9_configure(AVCodecContext *avctx) > >> priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100; > >> } > >> > >> + ctx->roi_quant_range = VP9_MAX_QUANT; > >> + > >> return 0; > >> } > >> > >> -- > >> 2.19.2 > >> > > Thanks, > > - Mark > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Wednesday, March 13, 2019 08:18 > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH v2 5/6] vaapi_encode: Add ROI support > > --- > libavcodec/vaapi_encode.c | 128 > ++++++++++++++++++++++++++++++++ > libavcodec/vaapi_encode.h | 11 +++ > libavcodec/vaapi_encode_h264.c | 2 + > libavcodec/vaapi_encode_h265.c | 2 + > libavcodec/vaapi_encode_mpeg2.c | 2 + > libavcodec/vaapi_encode_vp8.c | 2 + > libavcodec/vaapi_encode_vp9.c | 2 + > 7 files changed, 149 insertions(+) > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 2dda451882..03a7f5ce3e 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext > *avctx, > int err, i; > char data[MAX_PARAM_BUFFER_SIZE]; > size_t bit_len; > + AVFrameSideData *sd; > > av_log(avctx, AV_LOG_DEBUG, "Issuing encode for > pic %"PRId64"/%"PRId64" " > "as type %s.\n", pic->display_order, pic->encode_order, > @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext > *avctx, > } > } > > + sd = av_frame_get_side_data(pic->input_image, > + AV_FRAME_DATA_REGIONS_OF_INTEREST); > + > +#if VA_CHECK_VERSION(1, 0, 0) > + if (sd && ctx->roi_allowed) { > + const AVRegionOfInterest *roi; > + int nb_roi; > + VAEncMiscParameterBuffer param_misc = { > + .type = VAEncMiscParameterTypeROI, > + }; > + VAEncMiscParameterBufferROI param_roi; > + // VAAPI requires the second structure to immediately follow the > + // first in the parameter buffer, but they have different natural > + // alignment on 64-bit architectures (4 vs. 8, since the ROI > + // structure contains a pointer). This means we can't make a > + // single working structure, nor can we make the buffer > + // separately and map it because dereferencing the second pointer > + // would be undefined. Therefore, construct the two parts > + // separately and combine them into a single character buffer > + // before uploading. > + char param_buffer[sizeof(param_misc) + sizeof(param_roi)]; > + int i, v; How about using packed attribute to cope with this? Like: struct { VAEncMiscParameterBuffer misc; VAEncMiscParameterBufferROI param_roi; } __attribute__((packed)) mfs_params; This happens a lot during vaapi feature development, like MaxFrameSize: struct { VAEncMiscParameterBuffer misc; VAEncMiscParameterBufferMultiPassFrameSize mfs; } __attribute__((packed)) mfs_params; Trellis: (currently waiting for bug fix in libva) https://patchwork.ffmpeg.org/patch/11733/ Thanks, Linjie
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 2dda451882..03a7f5ce3e 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -143,6 +143,7 @@ static int vaapi_encode_issue(AVCodecContext *avctx, int err, i; char data[MAX_PARAM_BUFFER_SIZE]; size_t bit_len; + AVFrameSideData *sd; av_log(avctx, AV_LOG_DEBUG, "Issuing encode for pic %"PRId64"/%"PRId64" " "as type %s.\n", pic->display_order, pic->encode_order, @@ -412,6 +413,91 @@ static int vaapi_encode_issue(AVCodecContext *avctx, } } + sd = av_frame_get_side_data(pic->input_image, + AV_FRAME_DATA_REGIONS_OF_INTEREST); + +#if VA_CHECK_VERSION(1, 0, 0) + if (sd && ctx->roi_allowed) { + const AVRegionOfInterest *roi; + int nb_roi; + VAEncMiscParameterBuffer param_misc = { + .type = VAEncMiscParameterTypeROI, + }; + VAEncMiscParameterBufferROI param_roi; + // VAAPI requires the second structure to immediately follow the + // first in the parameter buffer, but they have different natural + // alignment on 64-bit architectures (4 vs. 8, since the ROI + // structure contains a pointer). This means we can't make a + // single working structure, nor can we make the buffer + // separately and map it because dereferencing the second pointer + // would be undefined. Therefore, construct the two parts + // separately and combine them into a single character buffer + // before uploading. + char param_buffer[sizeof(param_misc) + sizeof(param_roi)]; + int i, v; + + roi = (const AVRegionOfInterest*)sd->data; + av_assert0(roi->self_size && sd->size % roi->self_size == 0); + nb_roi = sd->size / roi->self_size; + if (nb_roi > ctx->roi_regions) { + if (!ctx->roi_warned) { + av_log(avctx, AV_LOG_WARNING, "More ROIs set than " + "supported by driver (%d > %d).\n", + nb_roi, ctx->roi_regions); + ctx->roi_warned = 1; + } + nb_roi = ctx->roi_regions; + } + + pic->roi = av_mallocz_array(nb_roi, sizeof(*pic->roi)); + if (!pic->roi) { + err = AVERROR(ENOMEM); + goto fail; + } + for (i = 0; i < nb_roi; i++) { + roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i); + + v = roi->qoffset.num * ctx->roi_quant_range / roi->qoffset.den; + av_log(avctx, AV_LOG_DEBUG, "ROI region: (%d,%d) (%dx%d) -> %+d.\n", + roi->x, roi->y, roi->width, roi->height, v); + + pic->roi[i] = (VAEncROI) { + .roi_rectangle = { + .x = roi->x, + .y = roi->y, + .width = roi->width, + .height = roi->height, + }, + .roi_value = av_clip_c(v, INT8_MIN, INT8_MAX), + }; + } + + param_roi = (VAEncMiscParameterBufferROI) { + .num_roi = nb_roi, + .max_delta_qp = INT8_MAX, + .min_delta_qp = 0, + .roi = pic->roi, + .roi_flags.bits.roi_value_is_qp_delta = 1, + }; + + memcpy(param_buffer, ¶m_misc, sizeof(param_misc)); + memcpy(param_buffer + sizeof(param_misc), + ¶m_roi, sizeof(param_roi)); + + err = vaapi_encode_make_param_buffer(avctx, pic, + VAEncMiscParameterBufferType, + param_buffer, + sizeof(param_buffer)); + if (err < 0) + goto fail; + } else +#endif + if (sd && !ctx->roi_warned) { + av_log(avctx, AV_LOG_WARNING, "ROI side data on input " + "frames ignored due to lack of driver support.\n"); + ctx->roi_warned = 1; + } + vas = vaBeginPicture(ctx->hwctx->display, ctx->va_context, pic->input_surface); if (vas != VA_STATUS_SUCCESS) { @@ -477,6 +563,7 @@ fail_at_end: av_freep(&pic->codec_picture_params); av_freep(&pic->param_buffers); av_freep(&pic->slices); + av_freep(&pic->roi); av_frame_free(&pic->recon_image); av_buffer_unref(&pic->output_buffer_ref); pic->output_buffer = VA_INVALID_ID; @@ -611,6 +698,7 @@ static int vaapi_encode_free(AVCodecContext *avctx, av_freep(&pic->priv_data); av_freep(&pic->codec_picture_params); + av_freep(&pic->roi); av_free(pic); @@ -1899,6 +1987,42 @@ static av_cold int vaapi_encode_init_quality(AVCodecContext *avctx) return 0; } +static av_cold int vaapi_encode_init_roi(AVCodecContext *avctx) +{ + VAAPIEncodeContext *ctx = avctx->priv_data; + +#if VA_CHECK_VERSION(1, 0, 0) + VAStatus vas; + VAConfigAttrib attr = { VAConfigAttribEncROI }; + + 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 ROI " + "config attribute: %d (%s).\n", vas, vaErrorStr(vas)); + return AVERROR_EXTERNAL; + } + + if (attr.value == VA_ATTRIB_NOT_SUPPORTED) { + ctx->roi_allowed = 0; + } else { + VAConfigAttribValEncROI roi = { + .value = attr.value, + }; + + ctx->roi_regions = roi.bits.num_roi_regions; + ctx->roi_allowed = ctx->roi_regions > 0 && + (ctx->va_rc_mode == VA_RC_CQP || + roi.bits.roi_rc_qp_delta_support); + } +#else + ctx->roi_allowed = 0; +#endif + return 0; +} + static void vaapi_encode_free_output_buffer(void *opaque, uint8_t *data) { @@ -2089,6 +2213,10 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx) if (err < 0) goto fail; + err = vaapi_encode_init_roi(avctx); + if (err < 0) + goto fail; + if (avctx->compression_level >= 0) { err = vaapi_encode_init_quality(avctx); if (err < 0) diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h index 44a8db566e..ee074b4dc1 100644 --- a/libavcodec/vaapi_encode.h +++ b/libavcodec/vaapi_encode.h @@ -69,6 +69,11 @@ typedef struct VAAPIEncodePicture { int64_t pts; int force_idr; +#if VA_CHECK_VERSION(1, 0, 0) + // ROI regions. + VAEncROI *roi; +#endif + int type; int b_depth; int encode_issued; @@ -314,6 +319,12 @@ typedef struct VAAPIEncodeContext { int idr_counter; int gop_counter; int end_of_stream; + + // ROI state. + int roi_allowed; + int roi_regions; + int roi_quant_range; + int roi_warned; } VAAPIEncodeContext; enum { diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c index 91be33f99f..7c55b8a4a7 100644 --- a/libavcodec/vaapi_encode_h264.c +++ b/libavcodec/vaapi_encode_h264.c @@ -1123,6 +1123,8 @@ static av_cold int vaapi_encode_h264_configure(AVCodecContext *avctx) } } + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); + return 0; } diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c index 758bd40a37..538862a9d5 100644 --- a/libavcodec/vaapi_encode_h265.c +++ b/libavcodec/vaapi_encode_h265.c @@ -1102,6 +1102,8 @@ static av_cold int vaapi_encode_h265_configure(AVCodecContext *avctx) priv->fixed_qp_b = 30; } + ctx->roi_quant_range = 51 + 6 * (ctx->profile->depth - 8); + return 0; } diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c index fb1ef71fdc..bac9ea1fa6 100644 --- a/libavcodec/vaapi_encode_mpeg2.c +++ b/libavcodec/vaapi_encode_mpeg2.c @@ -552,6 +552,8 @@ static av_cold int vaapi_encode_mpeg2_configure(AVCodecContext *avctx) ctx->nb_slices = ctx->slice_block_rows; ctx->slice_size = 1; + ctx->roi_quant_range = 31; + return 0; } diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c index ddbe4c9075..6e7bf9d106 100644 --- a/libavcodec/vaapi_encode_vp8.c +++ b/libavcodec/vaapi_encode_vp8.c @@ -173,6 +173,8 @@ static av_cold int vaapi_encode_vp8_configure(AVCodecContext *avctx) else priv->q_index_i = priv->q_index_p; + ctx->roi_quant_range = VP8_MAX_QUANT; + return 0; } diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c index f89fd0d07a..d7f415d704 100644 --- a/libavcodec/vaapi_encode_vp9.c +++ b/libavcodec/vaapi_encode_vp9.c @@ -202,6 +202,8 @@ static av_cold int vaapi_encode_vp9_configure(AVCodecContext *avctx) priv->q_idx_idr = priv->q_idx_p = priv->q_idx_b = 100; } + ctx->roi_quant_range = VP9_MAX_QUANT; + return 0; }