Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Tuesday, March 05, 2019 8:26 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support > > On 28/02/2019 06:33, Guo, Yejun wrote:>> -Original Message- > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > >> Of Mark Thompson > >> Sent: Thursday, February 28, 2019 6:00 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support > >> > >> --- > >> libavcodec/vaapi_encode.c | 129 > >> > >> libavcodec/vaapi_encode.h | 9 +++ > >> 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, 148 insertions(+) > > > > I tried this patch with below command, but do not see any quality change > with my eyes. > > I debugged in gdb and found the ROI data are correct in function > vaapi_encode_issue. > > > > I do not investigate deeper, and just want to first confirm if you see the > quality change or not. I might did something basic wrong. > > > > ffmpeg_g -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -s 1920x1080 > -i ../video > > /1080P_park_joy_1920x1080_500frames.yuv -vf format=nv12,hwupload > -c:v h264_vaapi -b:v 3M -y tmp.264 > > (my trick code in vf_scale.c is called with the above command) > > > > I tried the similar option with libx264 and found obvious video quality > changes. > > If you are using the i965 driver then you might need > <https://github.com/intel/intel-vaapi-driver/pull/447> to make it work. The > iHD driver worked for me with no changes. > > In CQP mode with H.264 it's straightforward to verify the output directly, too > - any stream analyser or other tool which can show the QP on each > macroblock will make it very obvious, since you will see exactly the offset > you > set in your regions of interest. (The reference decoder with trace enabled > shows it as mb_qp_delta as well.) yes, CQP mode is more straightforward. I can see the difference obviously with my eyes when using option "-qp 50", and yes, with the analyzer tool, I can see each MB's final qp is set as expected, but do not see the difference in 'slice_qp_delta' and 'mb_qp_delta' in the analyzer. > > Thanks, > > - Mark > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
On 28/02/2019 06:33, Guo, Yejun wrote:>> -Original Message- >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf >> Of Mark Thompson >> Sent: Thursday, February 28, 2019 6:00 AM >> To: ffmpeg-devel@ffmpeg.org >> Subject: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support >> >> --- >> libavcodec/vaapi_encode.c | 129 >> >> libavcodec/vaapi_encode.h | 9 +++ >> 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, 148 insertions(+) > > I tried this patch with below command, but do not see any quality change with > my eyes. > I debugged in gdb and found the ROI data are correct in function > vaapi_encode_issue. > > I do not investigate deeper, and just want to first confirm if you see the > quality change or not. I might did something basic wrong. > > ffmpeg_g -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -s 1920x1080 -i > ../video > /1080P_park_joy_1920x1080_500frames.yuv -vf format=nv12,hwupload-c:v > h264_vaapi -b:v 3M -y tmp.264 > (my trick code in vf_scale.c is called with the above command) > > I tried the similar option with libx264 and found obvious video quality > changes. If you are using the i965 driver then you might need <https://github.com/intel/intel-vaapi-driver/pull/447> to make it work. The iHD driver worked for me with no changes. In CQP mode with H.264 it's straightforward to verify the output directly, too - any stream analyser or other tool which can show the QP on each macroblock will make it very obvious, since you will see exactly the offset you set in your regions of interest. (The reference decoder with trace enabled shows it as mb_qp_delta as well.) Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Michael Niedermayer > Sent: Friday, March 01, 2019 6:13 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support > > On Wed, Feb 27, 2019 at 10:00:22PM +, Mark Thompson wrote: > > --- > > libavcodec/vaapi_encode.c | 129 > > > libavcodec/vaapi_encode.h | 9 +++ > > 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, 148 insertions(+) > > In file included from libavcodec/vaapi_encode.c:27:0: > libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’ > VAEncROI *roi; > ^ > make: *** [libavcodec/vaapi_encode.o] Error 1 looks that we'd add a libva version check, this struct is defined at libva repo. ./va/va.h:2259:} VAEncROI; I yesterday just did 'git pull' for the following repos: https://github.com/intel/libva https://github.com/intel/gmmlib.git https://github.com/intel/media-driver.git > make: *** Waiting for unfinished jobs > In file included from libavcodec/vaapi_encode_mpeg2.c:28:0: > libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’ > VAEncROI *roi; > ^ > make: *** [libavcodec/vaapi_encode_mpeg2.o] Error 1 > POD doc/ffprobe.pod > In file included from libavcodec/vaapi_encode_h264.c:36:0: > libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’ > VAEncROI *roi; > ^ > make: *** [libavcodec/vaapi_encode_h264.o] Error 1 > > > > [...] > -- > Michael GnuPG fingerprint: > 9FF2128B147EF6730BADF133611EC787040B0FAB > > Dictatorship: All citizens are under surveillance, all their steps and > actions recorded, for the politicians to enforce control. > Democracy: All politicians are under surveillance, all their steps and > actions recorded, for the citizens to enforce control. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
On Wed, Feb 27, 2019 at 10:00:22PM +, Mark Thompson wrote: > --- > libavcodec/vaapi_encode.c | 129 > libavcodec/vaapi_encode.h | 9 +++ > 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, 148 insertions(+) In file included from libavcodec/vaapi_encode.c:27:0: libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’ VAEncROI *roi; ^ make: *** [libavcodec/vaapi_encode.o] Error 1 make: *** Waiting for unfinished jobs In file included from libavcodec/vaapi_encode_mpeg2.c:28:0: libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’ VAEncROI *roi; ^ make: *** [libavcodec/vaapi_encode_mpeg2.o] Error 1 POD doc/ffprobe.pod In file included from libavcodec/vaapi_encode_h264.c:36:0: libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’ VAEncROI *roi; ^ make: *** [libavcodec/vaapi_encode_h264.o] Error 1 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship: All citizens are under surveillance, all their steps and actions recorded, for the politicians to enforce control. Democracy: All politicians are under surveillance, all their steps and actions recorded, for the citizens to enforce control. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Thursday, February 28, 2019 6:00 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
>
> ---
> libavcodec/vaapi_encode.c | 129
>
> libavcodec/vaapi_encode.h | 9 +++
> 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, 148 insertions(+)
I tried this patch with below command, but do not see any quality change with
my eyes.
I debugged in gdb and found the ROI data are correct in function
vaapi_encode_issue.
I do not investigate deeper, and just want to first confirm if you see the
quality change or not. I might did something basic wrong.
ffmpeg_g -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -s 1920x1080 -i
../video
/1080P_park_joy_1920x1080_500frames.yuv -vf format=nv12,hwupload-c:v
h264_vaapi -b:v 3M -y tmp.264
(my trick code in vf_scale.c is called with the above command)
I tried the similar option with libx264 and found obvious video quality changes.
>
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 2dda451882..1865006c1a 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,92 @@ 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,%d-%d) -
> > %+d.\n",
> + roi->left, avctx->width - roi->right,
> + roi->top, avctx->height - roi->bottom, v);
> +
> +pic->roi[i] = (VAEncROI) {
> +.roi_rectangle = {
> +.x = roi->left,
> +.y = roi->top,
> +.width = avctx->width - roi->right - roi->left,
> +.height = avctx->height - roi-
[FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
---
libavcodec/vaapi_encode.c | 129
libavcodec/vaapi_encode.h | 9 +++
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, 148 insertions(+)
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2dda451882..1865006c1a 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,92 @@ 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,%d-%d) -> %+d.\n",
+ roi->left, avctx->width - roi->right,
+ roi->top, avctx->height - roi->bottom, v);
+
+pic->roi[i] = (VAEncROI) {
+.roi_rectangle = {
+.x = roi->left,
+.y = roi->top,
+.width = avctx->width - roi->right - roi->left,
+.height = avctx->height - roi->bottom - roi->top,
+},
+.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, _misc, sizeof(param_misc));
+memcpy(param_buffer + sizeof(param_misc),
+ _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 +564,7 @@ fail_at_end:
av_freep(>codec_picture_params);
av_freep(>param_buffers);
av_freep(>slices);
+av_freep(>roi);
av_frame_free(>recon_image);
av_buffer_unref(>output_buffer_ref);
pic->output_buffer = VA_INVALID_ID;
@@ -611,6 +699,7 @@ static int vaapi_encode_free(AVCodecContext *avctx,
