On 27/03/2019 10:27, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong...@intel.com>
> ---
> V2: Fix the regression of qsv h264 encoding since no VPS for h264
>
> libavcodec/qsvenc.c | 53 ++++++++++++++++++++++++++++++++++------
> libavcodec/qsvenc.h | 3 +++
> libavcodec/qsvenc_hevc.c | 10 +++++---
> 3 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 576d88c259..2f128597db 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -810,6 +810,16 @@ static int qsv_retrieve_enc_params(AVCodecContext
> *avctx, QSVEncContext *q)
> };
> #endif
>
> +#if QSV_HAVE_CO_VPS
> + uint8_t vps_buf[128];
Is this necessarily enough? A VPS can be large when it defines sublayers.
> + mfxExtCodingOptionVPS extradata_vps = {
> + .Header.BufferId = MFX_EXTBUFF_CODING_OPTION_VPS,
> + .Header.BufferSz = sizeof(extradata_vps),
> + .VPSBuffer = vps_buf,
> + .VPSBufSize = sizeof(vps_buf),
> + };
> +#endif
> +
> mfxExtBuffer *ext_buffers[] = {
> (mfxExtBuffer*)&extradata,
> (mfxExtBuffer*)&co,
> @@ -818,14 +828,24 @@ static int qsv_retrieve_enc_params(AVCodecContext
> *avctx, QSVEncContext *q)
> #endif
> #if QSV_HAVE_CO3
> (mfxExtBuffer*)&co3,
> +#endif
> +#if QSV_HAVE_CO_VPS
> + (mfxExtBuffer*)&extradata_vps,
> #endif
> };
>
> int need_pps = avctx->codec_id != AV_CODEC_ID_MPEG2VIDEO;
> - int ret;
> + int ret, extradata_offset = 0;
>
> q->param.ExtParam = ext_buffers;
> +
> +#if QSV_HAVE_CO_VPS
> + q->hevc_vps = ((avctx->codec_id == AV_CODEC_ID_HEVC) &&
> QSV_RUNTIME_VERSION_ATLEAST(q->ver, 1, 17));
> + q->param.NumExtParam = FF_ARRAY_ELEMS(ext_buffers) - 1 + q->hevc_vps;
The array definition and then this length feels a bit overly tricky - any
change here will be quite confusing (consider adding a new ExtBuffer).
Making ext_buffers large enough with a running nb_ext_buffers total and then
adding the extra one only if H.265 would feel safer to me?
> +#else
> + q->hevc_vps = 0;
> q->param.NumExtParam = FF_ARRAY_ELEMS(ext_buffers);
> +#endif
>
> ret = MFXVideoENCODE_GetVideoParam(q->session, &q->param);
> if (ret < 0)
> @@ -834,20 +854,37 @@ static int qsv_retrieve_enc_params(AVCodecContext
> *avctx, QSVEncContext *q)
>
> q->packet_size = q->param.mfx.BufferSizeInKB *
> q->param.mfx.BRCParamMultiplier * 1000;
>
> - if (!extradata.SPSBufSize || (need_pps && !extradata.PPSBufSize)) {
> + if (!extradata.SPSBufSize || (need_pps && !extradata.PPSBufSize)
> +#if QSV_HAVE_CO_VPS
> + || (q->hevc_vps && !extradata_vps.VPSBufSize)
> +#endif
> + ) {
> av_log(avctx, AV_LOG_ERROR, "No extradata returned from libmfx.\n");
> return AVERROR_UNKNOWN;
> }
>
> - avctx->extradata = av_malloc(extradata.SPSBufSize + need_pps *
> extradata.PPSBufSize +
> - AV_INPUT_BUFFER_PADDING_SIZE);
> + avctx->extradata_size = extradata.SPSBufSize + need_pps *
> extradata.PPSBufSize;
> +#if QSV_HAVE_CO_VPS
> + avctx->extradata_size += q->hevc_vps * extradata_vps.VPSBufSize;
> +#endif
> +
> + avctx->extradata = av_malloc(avctx->extradata_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> if (!avctx->extradata)
> return AVERROR(ENOMEM);
>
> - memcpy(avctx->extradata, sps_buf,
> extradata.SPSBufSize);
> - if (need_pps)
> - memcpy(avctx->extradata + extradata.SPSBufSize, pps_buf,
> extradata.PPSBufSize);
> - avctx->extradata_size = extradata.SPSBufSize + need_pps *
> extradata.PPSBufSize;
> +#if QSV_HAVE_CO_VPS
> + if (q->hevc_vps) {
> + memcpy(avctx->extradata, vps_buf, extradata_vps.VPSBufSize);
> + extradata_offset += extradata_vps.VPSBufSize;
> + }
> +#endif
> +
> + memcpy(avctx->extradata + extradata_offset, sps_buf,
> extradata.SPSBufSize);
> + extradata_offset += extradata.SPSBufSize;
> + if (need_pps) {
> + memcpy(avctx->extradata + extradata_offset, pps_buf,
> extradata.PPSBufSize);
> + extradata_offset += extradata.PPSBufSize;
> + }
> memset(avctx->extradata + avctx->extradata_size, 0,
> AV_INPUT_BUFFER_PADDING_SIZE);
>
> cpb_props = ff_add_cpb_side_data(avctx);
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index 25105894f2..f01453e67f 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -36,6 +36,7 @@
>
> #define QSV_HAVE_CO2 QSV_VERSION_ATLEAST(1, 6)
> #define QSV_HAVE_CO3 QSV_VERSION_ATLEAST(1, 11)
> +#define QSV_HAVE_CO_VPS QSV_VERSION_ATLEAST(1, 17)
>
> #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
> #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
> @@ -135,6 +136,8 @@ typedef struct QSVEncContext {
>
> mfxVersion ver;
>
> + int hevc_vps;
> +
> // options set by the caller
> int async_depth;
> int idr_interval;
> diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
> index 9e15d3ce6b..df6ef24b72 100644
> --- a/libavcodec/qsvenc_hevc.c
> +++ b/libavcodec/qsvenc_hevc.c
> @@ -195,10 +195,12 @@ static av_cold int qsv_enc_init(AVCodecContext *avctx)
> if (ret < 0)
> return ret;
>
> - ret = generate_fake_vps(&q->qsv, avctx);
> - if (ret < 0) {
> - ff_qsv_enc_close(avctx, &q->qsv);
> - return ret;
> + if (!q->qsv.hevc_vps) {
> + ret = generate_fake_vps(&q->qsv, avctx);
> + if (ret < 0) {
> + ff_qsv_enc_close(avctx, &q->qsv);
> + return ret;
> + }
> }
>
> return 0;
>
Rest looks good.
Thanks,
- 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-requ...@ffmpeg.org with subject "unsubscribe".
