thanks for the feedbacks , below some answers and v2 is upcoming. On Thu, Mar 29, 2018 at 1:03 AM, Mark Thompson <[email protected]> wrote:
> On 27/03/18 23:07, Maxym Dmytrychenko wrote: > > On Tue, Mar 27, 2018 at 12:47 AM, Mark Thompson <[email protected]> wrote: > > > >> On 26/03/18 21:18, Maxym Dmytrychenko wrote: > >>> Starting from API 1.25 helps to improve performance of the simultaneous > >> encode, > >>> 1:N scenario, like: > >>> > >>> ./avconv -y -hwaccel qsv -c:v h264_qsv -r 30000/1001 -i > >>> ~/bbb_sunflower_1080p_60fps_normal.mp4 -vframes 600 -an \ > >>> -filter_complex "split=2[s1][s2]; [s1]scale_qsv=1280:720[o1]; > >>> [s2]scale_qsv=960:540[o2]" \ > >>> -map [o1] -c:v h264_qsv -b:v 3200k -minrate 3200k -maxrate 3200k -f > >>> rawvideo /tmp/3200a.264 \ > >>> -map [o2] -c:v h264_qsv -b:v 1750k -minrate 1750k -maxrate 1750k -f > >>> rawvideo /tmp/1750a.264 > >> > >> Having seen this interface being added form the VAAPI side, I got the > >> impression that this only makes sense if the parallel streams are > actually > >> in lock-step (as in your example). What will happen if this isn't the > case? > >> > > thanks for the points, Mark. > > it is actually testing phase but can you share more details over > example(s) > > you mean? > > Well, your example has two streams which encode exactly in parallel - in > every step both streams encode one frame. > > I don't know what the implementation inside libmfx is, but suppose the > streams don't run at the same rate, e.g. if one stream is twice as fast as > the other as in (with libx264): > > ./avconv -y -i in.mp4 -an -filter_complex 'split=2[a][b]' -map '[a]' -r 10 > -c:v libx264 out_10.mp4 -map '[b]' -r 20 -c:v libx264 out_20.mp4 > > Does everything still work as expected? > > As a worse case, suppose you have multiple input streams with unknown > properties (including framerate) which want to be encoded with low-latency, > does setting this option affect that latency because some streams wait for > new frames to appear on others? > > this option in WIP now but expectations - latency should not be affected > (These questions are primarily trying to assess whether this should be > enabled by default.) > > >> I'm also curious what the gain from this is, if you happen to have some > >> numbers... > >> > > Actually depends on exact HW SKU and resolution/scenario, however, can be > > well above 10% > > > > > >>> --- > >>> libavcodec/qsv.c | 10 ++++++---- > >>> libavcodec/qsv_internal.h | 3 +++ > >>> libavcodec/qsvdec.c | 1 - > >>> libavcodec/qsvenc.c | 16 +++++++++++++++- > >>> libavcodec/qsvenc.h | 12 ++++++++++-- > >>> libavcodec/qsvenc_h264.c | 4 ++++ > >>> libavfilter/qsvvpp.c | 9 ++++++--- > >>> libavfilter/qsvvpp.h | 7 +++++++ > >>> libavfilter/vf_deinterlace_qsv.c | 6 ++++++ > >>> libavfilter/vf_scale_qsv.c | 6 ++++++ > >>> libavutil/hwcontext_qsv.c | 5 +++++ > >>> libavutil/hwcontext_qsv.h | 3 +++ > >>> 12 files changed, 71 insertions(+), 11 deletions(-) > >>> > >>> diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c > >>> index e78633d62..bab32836e 100644 > >>> --- a/libavcodec/qsv.c > >>> +++ b/libavcodec/qsv.c > >>> @@ -593,10 +593,12 @@ int ff_qsv_init_session_device(AVCodecContext > >> *avctx, mfxSession *psession, > >>> "Error setting a HW handle"); > >>> } > >>> > >>> - err = MFXJoinSession(parent_session, session); > >>> - if (err != MFX_ERR_NONE) > >>> - return ff_qsv_print_error(avctx, err, > >>> - "Error joining session"); > >>> + if (QSV_RUNTIME_VERSION_ATLEAST(ver, 1, 25)) { > >>> + err = MFXJoinSession(parent_session, session); > >>> + if (err != MFX_ERR_NONE) > >>> + return ff_qsv_print_error(avctx, err, > >>> + "Error joining session"); > >>> + } > >>> > >>> ret = qsv_load_plugins(session, load_plugins, avctx); > >>> if (ret < 0) { > >>> diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h > >>> index 975c8de44..516994a64 100644 > >>> --- a/libavcodec/qsv_internal.h > >>> +++ b/libavcodec/qsv_internal.h > >>> @@ -36,6 +36,9 @@ > >>> (MFX_VERSION_MAJOR > (MAJOR) || \ > >>> MFX_VERSION_MAJOR == (MAJOR) && MFX_VERSION_MINOR >= (MINOR)) > >>> > >>> +#define QSV_RUNTIME_VERSION_ATLEAST(MFX_VERSION, MAJOR, MINOR) \ > >>> + (MFX_VERSION.Major == MAJOR && MFX_VERSION.Minor >= MINOR) > >>> + > >>> typedef struct QSVMid { > >>> AVBufferRef *hw_frames_ref; > >>> mfxHDL handle; > >>> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c > >>> index f31172de2..b45216291 100644 > >>> --- a/libavcodec/qsvdec.c > >>> +++ b/libavcodec/qsvdec.c > >>> @@ -332,7 +332,6 @@ static int qsv_decode(AVCodecContext *avctx, > >> QSVContext *q, > >>> av_freep(&sync); > >>> return ret; > >>> } > >>> - > >> > >> Stray change. > >> > > + per Luca's comment - will remove it. > > > > > >> > >>> ret = MFXVideoDECODE_DecodeFrameAsync(q->session, avpkt->size > >> ? &bs : NULL, > >>> insurf, &outsurf, sync); > >>> if (ret == MFX_WRN_DEVICE_BUSY) > >>> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > >>> index f6b1a0d67..a8b446c5b 100644 > >>> --- a/libavcodec/qsvenc.c > >>> +++ b/libavcodec/qsvenc.c > >>> @@ -135,7 +135,7 @@ static void dump_video_param(AVCodecContext > *avctx, > >> QSVEncContext *q, > >>> #if QSV_HAVE_CO2 > >>> mfxExtCodingOption2 *co2 = (mfxExtCodingOption2*)coding_opts[1]; > >>> #endif > >>> -#if QSV_HAVE_CO3 > >>> +#if QSV_HAVE_CO3 && QSV_HAVE_QVBR > >>> mfxExtCodingOption3 *co3 = (mfxExtCodingOption3*)coding_opts[2]; > >>> #endif > >>> > >>> @@ -656,6 +656,20 @@ FF_ENABLE_DEPRECATION_WARNINGS > >>> > >>> q->extparam_internal[q->nb_extparam_internal++] = > >> (mfxExtBuffer *)&q->extco2; > >>> } > >>> +#endif > >>> +#if QSV_HAVE_MF > >>> + if (avctx->codec_id == AV_CODEC_ID_H264) { > >>> + mfxVersion ver; > >>> + ret = MFXQueryVersion(q->session,&ver); > >>> + if (ret >= MFX_ERR_NONE && QSV_RUNTIME_VERSION_ATLEAST( > ver, > >> 1, 25)) { > >>> + q->extmfp.Header.BufferId = > >> MFX_EXTBUFF_MULTI_FRAME_PARAM; > >>> + q->extmfp.Header.BufferSz = sizeof(q->extmfp); > >>> + > >>> + q->extmfp.MFMode = q->mfmode; > >>> + av_log(avctx,AV_LOG_VERBOSE,"MFMode:%d\n", > >> q->extmfp.MFMode); > >>> + q->extparam_internal[q->nb_extparam_internal++] = > >> (mfxExtBuffer *)&q->extmfp; > >>> + } > >>> + } > >> > >> Why is this conditioned on H.264? The API won't be different for other > >> codecs, will it? > >> > > only H.264 > > And the API rejects it being passed to other codecs? > > yes > >>> #endif > >>> } > >>> > >>> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h > >>> index ab5579595..a7fc57bb4 100644 > >>> --- a/libavcodec/qsvenc.h > >>> +++ b/libavcodec/qsvenc.h > >>> @@ -50,11 +50,13 @@ > >>> #define QSV_HAVE_ICQ QSV_VERSION_ATLEAST(1, 8) > >>> #define QSV_HAVE_VCM QSV_VERSION_ATLEAST(1, 8) > >>> #define QSV_HAVE_QVBR QSV_VERSION_ATLEAST(1, 11) > >>> +#define QSV_HAVE_MF 0 > >>> #else > >>> #define QSV_HAVE_AVBR 0 > >>> #define QSV_HAVE_ICQ 0 > >>> #define QSV_HAVE_VCM 0 > >>> #define QSV_HAVE_QVBR 0 > >>> +#define QSV_HAVE_MF QSV_VERSION_ATLEAST(1, 25) > >>> #endif > >>> > >>> #if !QSV_HAVE_LA_DS > >>> @@ -109,12 +111,15 @@ typedef struct QSVEncContext { > >>> #if QSV_HAVE_CO2 > >>> mfxExtCodingOption2 extco2; > >>> #endif > >>> - > >>> +#if QSV_HAVE_MF > >>> + mfxExtMultiFrameParam extmfp; > >>> + mfxExtMultiFrameControl extmfc; > >>> +#endif > >>> mfxExtOpaqueSurfaceAlloc opaque_alloc; > >>> mfxFrameSurface1 **opaque_surfaces; > >>> AVBufferRef *opaque_alloc_buf; > >>> > >>> - mfxExtBuffer *extparam_internal[2 + QSV_HAVE_CO2]; > >>> + mfxExtBuffer *extparam_internal[2 + QSV_HAVE_CO2 + (QSV_HAVE_MF * > >> 2)]; > >>> int nb_extparam_internal; > >>> > >>> mfxExtBuffer **extparam; > >>> @@ -156,6 +161,9 @@ typedef struct QSVEncContext { > >>> int int_ref_qp_delta; > >>> int recovery_point_sei; > >>> > >>> +#if QSV_HAVE_MF > >>> + int mfmode; > >>> +#endif > >>> char *load_plugins; > >>> } QSVEncContext; > >>> > >>> diff --git a/libavcodec/qsvenc_h264.c b/libavcodec/qsvenc_h264.c > >>> index 634a7d3f9..ae00ff8d5 100644 > >>> --- a/libavcodec/qsvenc_h264.c > >>> +++ b/libavcodec/qsvenc_h264.c > >>> @@ -93,6 +93,10 @@ static const AVOption options[] = { > >>> > >>> { "aud", "Insert the Access Unit Delimiter NAL", OFFSET(qsv.aud), > >> AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE}, > >>> > >>> +#if QSV_HAVE_MF > >>> + { "mfmode", "Multi-Frame Mode", OFFSET(qsv.mfmode), > >> AV_OPT_TYPE_INT, { .i64 = MFX_MF_AUTO }, 0, INT_MAX, VE }, > >>> +#endif > >>> + > >>> { NULL }, > >>> }; > >>> > >>> diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c > >>> index a96cfa65d..f704517ae 100644 > >>> --- a/libavfilter/qsvvpp.c > >>> +++ b/libavfilter/qsvvpp.c > >>> @@ -515,9 +515,12 @@ static int init_vpp_session(AVFilterContext > >> *avctx, QSVVPPContext *s) > >>> if (ret != MFX_ERR_NONE) > >>> return AVERROR_UNKNOWN; > >>> } > >>> - ret = MFXJoinSession(device_hwctx->session, s->session); > >>> - if (ret != MFX_ERR_NONE) > >>> - return AVERROR_UNKNOWN; > >>> + > >>> + if (QSV_RUNTIME_VERSION_ATLEAST(ver, 1, 25)) { > >>> + ret = MFXJoinSession(device_hwctx->session, s->session); > >>> + if (ret != MFX_ERR_NONE) > >>> + return AVERROR_UNKNOWN; > >>> + } > >>> > >>> if (IS_OPAQUE_MEMORY(s->in_mem_mode) || > >> IS_OPAQUE_MEMORY(s->out_mem_mode)) { > >>> s->opaque_alloc.In.Surfaces = s->surface_ptrs_in; > >>> diff --git a/libavfilter/qsvvpp.h b/libavfilter/qsvvpp.h > >>> index 082c0a899..68e38bd70 100644 > >>> --- a/libavfilter/qsvvpp.h > >>> +++ b/libavfilter/qsvvpp.h > >>> @@ -31,6 +31,13 @@ > >>> #define FF_INLINK_IDX(link) ((int)((link)->dstpad - > >> (link)->dst->input_pads)) > >>> #define FF_OUTLINK_IDX(link) ((int)((link)->srcpad - > >> (link)->src->output_pads)) > >>> > >>> +#define QSV_VERSION_ATLEAST(MAJOR, MINOR) \ > >>> + (MFX_VERSION_MAJOR > (MAJOR) || \ > >>> + MFX_VERSION_MAJOR == (MAJOR) && MFX_VERSION_MINOR >= (MINOR)) > >>> + > >>> +#define QSV_RUNTIME_VERSION_ATLEAST(MFX_VERSION, MAJOR, MINOR) \ > >>> + (MFX_VERSION.Major == MAJOR && MFX_VERSION.Minor >= MINOR) > >>> + > >>> typedef struct QSVVPPContext QSVVPPContext; > >>> > >>> typedef struct QSVVPPCrop { > >>> diff --git a/libavfilter/vf_deinterlace_qsv.c > >> b/libavfilter/vf_deinterlace_qsv.c > >>> index 2360491d3..34bd15b1a 100644 > >>> --- a/libavfilter/vf_deinterlace_qsv.c > >>> +++ b/libavfilter/vf_deinterlace_qsv.c > >>> @@ -214,6 +214,12 @@ static int init_out_session(AVFilterContext *ctx) > >>> return AVERROR_UNKNOWN; > >>> } > >>> > >>> + if (QSV_RUNTIME_VERSION_ATLEAST(ver, 1, 25)) { > >>> + err = MFXJoinSession(device_hwctx->session, s->session); > >>> + if (err != MFX_ERR_NONE) > >>> + return AVERROR_UNKNOWN; > >>> + } > >>> + > >>> memset(&par, 0, sizeof(par)); > >>> > >>> s->deint_conf.Header.BufferId = MFX_EXTBUFF_VPP_DEINTERLACING; > >>> diff --git a/libavfilter/vf_scale_qsv.c b/libavfilter/vf_scale_qsv.c > >>> index c568e9625..e909cab23 100644 > >>> --- a/libavfilter/vf_scale_qsv.c > >>> +++ b/libavfilter/vf_scale_qsv.c > >>> @@ -313,6 +313,12 @@ static int init_out_session(AVFilterContext *ctx) > >>> return AVERROR_UNKNOWN; > >>> } > >>> > >>> + if (QSV_RUNTIME_VERSION_ATLEAST(ver, 1, 25)) { > >>> + err = MFXJoinSession(device_hwctx->session, s->session); > >>> + if (err != MFX_ERR_NONE) > >>> + return AVERROR_UNKNOWN; > >>> + } > >>> + > >>> memset(&par, 0, sizeof(par)); > >>> > >>> if (opaque) { > >>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c > >>> index f5d78d059..b3eb4a3ea 100644 > >>> --- a/libavutil/hwcontext_qsv.c > >>> +++ b/libavutil/hwcontext_qsv.c > >>> @@ -1058,6 +1058,11 @@ static int qsv_device_derive_from_child( > AVHWDeviceContext > >> *ctx, > >>> goto fail; > >>> } > >>> > >>> + ret = MFXQueryVersion(hwctx->session,&ver); > >>> + if (ret == MFX_ERR_NONE) { > >>> + av_log(ctx, AV_LOG_VERBOSE, "MFX compile/runtime API: > >> %d.%d/%d.%d\n", > >>> + MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, > >> ver.Minor); > >>> + } > >> > >> Should be a separate change? (But yes, good idea!) > >> > >> thanks, > > Just happed to be handy for this patch and related tests, would you > insist > > to have a separate patch ? > > Seems like it should be to me. > > >>> return 0; > >>> > >>> fail: > >>> diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h > >>> index 8f9d3d201..105448699 100644 > >>> --- a/libavutil/hwcontext_qsv.h > >>> +++ b/libavutil/hwcontext_qsv.h > >>> @@ -21,6 +21,9 @@ > >>> > >>> #include <mfx/mfxvideo.h> > >>> > >>> +#define QSV_RUNTIME_VERSION_ATLEAST(MFX_VERSION, MAJOR, MINOR) \ > >>> + (MFX_VERSION.Major == MAJOR && MFX_VERSION.Minor >= MINOR) > >> > >> This shouldn't be added to in the installed header. > >> > >> pls share more details/suggestions, so I can move it accordingly. > > This adds it to the public API of libavutil. You've already put > equivalent macros in private headers in the libraries, that should be > sufficient without this one being present at all I think? > > just compilation issue but pls see other .h usage in v2 patch > >>> + > >>> /** > >>> * @file > >>> * An API-specific header for AV_HWDEVICE_TYPE_QSV. > >>> > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
