[FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: expose qp of encoded frames
Li, Zhong
zhong.li at intel.com
Tue Aug 7 08:01:44 EEST 2018
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of James Almer
> Sent: Tuesday, August 7, 2018 12:14 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: expose qp of encoded
> frames
>
> On 8/7/2018 12:12 AM, James Almer wrote:
> > On 7/26/2018 4:51 AM, Zhong Li wrote:
> >> Requirement from ticket #7254.
> >> Currently only H264 supported by MSDK.
> >>
> >> Signed-off-by: Zhong Li <zhong.li at intel.com>
> >> ---
> >> libavcodec/qsvenc.c | 43
> +++++++++++++++++++++++++++++++++++++++++++
> >> libavcodec/qsvenc.h | 2 ++
> >> libavcodec/qsvenc_h264.c | 5 +++++
> >> 3 files changed, 50 insertions(+)
> >>
> >> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> >> 8096945..1294ed2 100644
> >> --- a/libavcodec/qsvenc.c
> >> +++ b/libavcodec/qsvenc.c
> >> @@ -1139,6 +1139,10 @@ static int encode_frame(AVCodecContext
> *avctx,
> >> QSVEncContext *q, {
> >> AVPacket new_pkt = { 0 };
> >> mfxBitstream *bs;
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + mfxExtAVCEncodedFrameInfo *enc_info;
> >> + mfxExtBuffer **enc_buf;
> >> +#endif
> >>
> >> mfxFrameSurface1 *surf = NULL;
> >> mfxSyncPoint *sync = NULL;
> >> @@ -1172,6 +1176,22 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >> bs->Data = new_pkt.data;
> >> bs->MaxLength = new_pkt.size;
> >>
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + if (avctx->codec_id == AV_CODEC_ID_H264) {
> >> + enc_info = av_mallocz(sizeof(*enc_info));
> >> + if (!enc_info)
> >> + return AVERROR(ENOMEM);
> >> +
> >> + enc_info->Header.BufferId =
> MFX_EXTBUFF_ENCODED_FRAME_INFO;
> >> + enc_info->Header.BufferSz = sizeof (*enc_info);
> >> + bs->NumExtParam = 1;
> >> + enc_buf = av_mallocz(sizeof(mfxExtBuffer *));
> >> + enc_buf[0] = (mfxExtBuffer *)enc_info;
> >> +
> >> + bs->ExtParam = enc_buf;
> >> + }
> >> +#endif
> >> +
> >> if (q->set_encode_ctrl_cb) {
> >> q->set_encode_ctrl_cb(avctx, frame, &qsv_frame->enc_ctrl);
> >> }
> >> @@ -1179,6 +1199,10 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >> sync = av_mallocz(sizeof(*sync));
> >> if (!sync) {
> >> av_freep(&bs);
> >> + #if QSV_VERSION_ATLEAST(1, 26)
> >> + if (avctx->codec_id == AV_CODEC_ID_H264)
> >> + av_freep(&enc_info);
> >> + #endif
> >> av_packet_unref(&new_pkt);
> >> return AVERROR(ENOMEM);
> >> }
> >> @@ -1195,6 +1219,10 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >> if (ret < 0) {
> >> av_packet_unref(&new_pkt);
> >> av_freep(&bs);
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + if (avctx->codec_id == AV_CODEC_ID_H264)
> >> + av_freep(&enc_info);
> >> +#endif
> >> av_freep(&sync);
> >> return (ret == MFX_ERR_MORE_DATA) ?
> >> 0 : ff_qsv_print_error(avctx, ret, "Error during
> >> encoding"); @@ -1211,6 +1239,10 @@ static int
> encode_frame(AVCodecContext *avctx, QSVEncContext *q,
> >> av_freep(&sync);
> >> av_packet_unref(&new_pkt);
> >> av_freep(&bs);
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + if (avctx->codec_id == AV_CODEC_ID_H264)
> >> + av_freep(&enc_info);
> >> +#endif
> >> }
> >>
> >> return 0;
> >> @@ -1230,6 +1262,9 @@ int ff_qsv_encode(AVCodecContext *avctx,
> QSVEncContext *q,
> >> AVPacket new_pkt;
> >> mfxBitstream *bs;
> >> mfxSyncPoint *sync;
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + mfxExtAVCEncodedFrameInfo *enc_info; #endif
> >>
> >> av_fifo_generic_read(q->async_fifo, &new_pkt,
> sizeof(new_pkt), NULL);
> >> av_fifo_generic_read(q->async_fifo, &sync, sizeof(sync),
> NULL);
> >> @@ -1258,6 +1293,14 @@ FF_DISABLE_DEPRECATION_WARNINGS
> >> FF_ENABLE_DEPRECATION_WARNINGS #endif
> >>
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + if (avctx->codec_id == AV_CODEC_ID_H264) {
> >> + enc_info = (mfxExtAVCEncodedFrameInfo
> *)(*bs->ExtParam);
> >> + av_log(avctx, AV_LOG_DEBUG, "QP is %d\n",
> enc_info->QP);
> >
> > I think you should use ff_side_data_set_encoder_stats() for this instead.
>
> I see you pushed this already. You didn't really give enough time after you
> said you'd wait for other comments, and even went and pushed it after i
> wrote the above email...
I meant the other patch: lavc/encode: fix frame_number double-counted. It is not just taking effect to qsv, but more common, so I haven't pushed it.
Anyway, thanks a lot for your review. And will update soon.
>
> Use ff_side_data_set_encoder_stats() to export frame quality encoding info
> instead of the above debug av_log() message as i requested. It's a trivial
> change, and the proper way to export such stats.
Agree, will keep alignment with other encoders such as libx264/nvenc.
>
> >
> >> + q->sum_frame_qp += enc_info->QP;
> >> + av_freep(&enc_info);
> >> + }
> >> +#endif
> >> av_freep(&bs);
> >> av_freep(&sync);
> >>
> >> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h index
> >> b2d6355..3784a82 100644
> >> --- a/libavcodec/qsvenc.h
> >> +++ b/libavcodec/qsvenc.h
> >> @@ -102,6 +102,8 @@ typedef struct QSVEncContext {
> >> int width_align;
> >> int height_align;
> >>
> >> + int sum_frame_qp;
> >> +
> >> mfxVideoParam param;
> >> mfxFrameAllocRequest req;
> >>
> >> diff --git a/libavcodec/qsvenc_h264.c b/libavcodec/qsvenc_h264.c
> >> index 5c262e5..b87bef6 100644
> >> --- a/libavcodec/qsvenc_h264.c
> >> +++ b/libavcodec/qsvenc_h264.c
> >> @@ -95,6 +95,11 @@ static av_cold int qsv_enc_close(AVCodecContext
> >> *avctx) {
> >> QSVH264EncContext *q = avctx->priv_data;
> >>
> >> +#if QSV_VERSION_ATLEAST(1, 26)
> >> + av_log(avctx, AV_LOG_VERBOSE, "encoded %d frames, avarge qp is
> >> +%.2f\n",
>
> average, not avarge.
Sure. Will update.
>
> >> + avctx->frame_number,(double)q->qsv.sum_frame_qp /
> >> +avctx->frame_number); #endif
> >> +
> >> return ff_qsv_enc_close(avctx, &q->qsv); }
> >
> >
>
More information about the ffmpeg-devel
mailing list