[FFmpeg-devel] [PATCH v5 0/6] Implement SEI parsing for QSV decoders
Soft Works
softworkz at hotmail.com
Fri Oct 21 10:42:48 EEST 2022
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Thursday, July 21, 2022 11:56 PM
> To: Xiang, Haihao <haihao.xiang at intel.com>; ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v5 0/6] Implement SEI parsing for
> QSV decoders
>
> Xiang, Haihao:
> > On Fri, 2022-07-01 at 20:48 +0000, ffmpegagent wrote:
> >> Missing SEI information has always been a major drawback when
> using the QSV
> >> decoders. I used to think that there's no chance to get at the
> data without
> >> explicit implementation from the MSDK side (or doing something
> weird like
> >> parsing in parallel). It turned out that there's a hardly known
> api method
> >> that provides access to all SEI (h264/hevc) or user data
> (mpeg2video).
> >>
> >> This allows to get things like closed captions, frame packing,
> display
> >> orientation, HDR data (mastering display, content light level,
> etc.) without
> >> having to rely on those data being provided by the MSDK as
> extended buffers.
> >>
> >> The commit "Implement SEI parsing for QSV decoders" includes some
> hard-coded
> >> workarounds for MSDK bugs which I reported:
> >>
> > https://github.com/Intel-Media-
> SDK/MediaSDK/issues/2597#issuecomment-1072795311
> >>
> >> But that doesn't help. Those bugs exist and I'm sharing my
> workarounds,
> >> which are empirically determined by testing a range of files. If
> someone is
> >> interested, I can provide private access to a repository where we
> have been
> >> testing this. Alternatively, I could also leave those workarounds
> out, and
> >> just skip those SEI types.
> >>
> >> In a previous version of this patchset, there was a concern that
> payload
> >> data might need to be re-ordered. Meanwhile I have researched this
> carefully
> >> and the conclusion is that this is not required.
> >>
> >> My detailed analysis can be found here:
> >> https://gist.github.com/softworkz/36c49586a8610813a32270ee3947a932
> >>
> >> v4
> >>
> >> * add new dependencies in makefile Now, build still works when
> someone uses
> >> configure --disable-decoder=h264 --disable-decoder=hevc
> >> --disable-decoder=mpegvideo --disable-decoder=mpeg1video
> >> --disable-decoder=mpeg2video --enable-libmfx
> >>
> >> v3
> >>
> >> * frame.h: clarify doc text for av_frame_copy_side_data()
> >>
> >> v2
> >>
> >> * qsvdec: make error handling consistent and clear
> >> * qsvdec: remove AV_CODEC_ID_MPEG1VIDEO constants
> >> * hevcdec: rename function to ff_hevc_set_side_data(), add doc
> text
> >>
> >> v3
> >>
> >> * qsvdec: fix c/p error
> >>
> >> softworkz (6):
> >> avutil/frame: Add av_frame_copy_side_data() and
> >> av_frame_remove_all_side_data()
> >> avcodec/vpp_qsv: Copy side data from input to output frame
> >> avcodec/mpeg12dec: make mpeg_decode_user_data() accessible
> >> avcodec/hevcdec: make set_side_data() accessible
> >> avcodec/h264dec: make h264_export_frame_props() accessible
> >> avcodec/qsvdec: Implement SEI parsing for QSV decoders
> >>
> >> doc/APIchanges | 4 +
> >> libavcodec/Makefile | 6 +-
> >> libavcodec/h264_slice.c | 98 ++++++++-------
> >> libavcodec/h264dec.h | 2 +
> >> libavcodec/hevcdec.c | 117 +++++++++---------
> >> libavcodec/hevcdec.h | 9 ++
> >> libavcodec/hevcdsp.c | 4 +
> >> libavcodec/mpeg12.h | 28 +++++
> >> libavcodec/mpeg12dec.c | 40 +-----
> >> libavcodec/qsvdec.c | 234
> +++++++++++++++++++++++++++++++++++
> >> libavfilter/qsvvpp.c | 6 +
> >> libavfilter/vf_overlay_qsv.c | 19 ++-
> >> libavutil/frame.c | 67 ++++++----
> >> libavutil/frame.h | 32 +++++
> >> libavutil/version.h | 2 +-
> >> 15 files changed, 494 insertions(+), 174 deletions(-)
> >>
> >>
> >> base-commit: 6a82412bf33108111eb3f63076fd5a51349ae114
> >> Published-As:
> >> https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 31%2Fsoftworkz%2Fsubmit_qsv_sei-v5
> >> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-
> >> 31/softworkz/submit_qsv_sei-v5
> >> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/31
> >>
> >> Range-diff vs v4:
> >>
> >> 1: 7656477360 = 1: 7656477360 avutil/frame: Add
> av_frame_copy_side_data()
> >> and av_frame_remove_all_side_data()
> >> 2: 06976606c5 = 2: 06976606c5 avcodec/vpp_qsv: Copy side data
> from input to
> >> output frame
> >> 3: 320a8a535c = 3: 320a8a535c avcodec/mpeg12dec: make
> >> mpeg_decode_user_data() accessible
> >> 4: e58ad6564f = 4: e58ad6564f avcodec/hevcdec: make
> set_side_data()
> >> accessible
> >> 5: a57bfaebb9 = 5: 4c0b6eb4cb avcodec/h264dec: make
> >> h264_export_frame_props() accessible
> >> 6: 3f2588563e ! 6: 19bc00be4d avcodec/qsvdec: Implement SEI
> parsing for QSV
> >> decoders
> >> @@ Commit message
> >>
> >> Signed-off-by: softworkz <softworkz at hotmail.com>
> >>
> >> + ## libavcodec/Makefile ##
> >> +@@ libavcodec/Makefile: OBJS-$(CONFIG_MSS34DSP)
> +=
> >> mss34dsp.o
> >> + OBJS-$(CONFIG_PIXBLOCKDSP) += pixblockdsp.o
> >> + OBJS-$(CONFIG_QPELDSP) += qpeldsp.o
> >> + OBJS-$(CONFIG_QSV) += qsv.o
> >> +-OBJS-$(CONFIG_QSVDEC) += qsvdec.o
> >> ++OBJS-$(CONFIG_QSVDEC) += qsvdec.o
> h264_slice.o
> >> h264_cabac.o h264_cavlc.o \
> >> ++ h264_direct.o
> h264_mb.o
> >> h264_picture.o h264_loopfilter.o \
> >> ++ h264dec.o
> h264_refs.o cabac.o
> >> hevcdec.o hevc_refs.o \
> >> ++
>
> >> hevc_filter.o hevc_cabac.o hevc_mvs.o hevcpred.o hevcdsp.o \
> >> ++
>
> >> h274.o dovi_rpu.o mpeg12dec.o
> >> + OBJS-$(CONFIG_QSVENC) += qsvenc.o
> >> + OBJS-$(CONFIG_RANGECODER) += rangecoder.o
> >> + OBJS-$(CONFIG_RDFT) += rdft.o
> >> +
> >> + ## libavcodec/hevcdsp.c ##
> >> +@@
> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor,
> Boston, MA 02110-
> >> 1301 USA
> >> + */
> >> +
> >> ++#include "config_components.h"
> >> ++
> >> + #include "hevcdsp.h"
> >> +
> >> + static const int8_t transform[32][32] = {
> >> +@@ libavcodec/hevcdsp.c: int i = 0;
> >> + break;
> >> + }
> >> +
> >> ++#if CONFIG_HEVC_DECODER
> >> + #if ARCH_AARCH64
> >> + ff_hevc_dsp_init_aarch64(hevcdsp, bit_depth);
> >> + #elif ARCH_ARM
> >> +@@ libavcodec/hevcdsp.c: int i = 0;
> >> + #elif ARCH_LOONGARCH
> >> + ff_hevc_dsp_init_loongarch(hevcdsp, bit_depth);
> >> + #endif
> >> ++#endif
> >> + }
> >> +
> >> ## libavcodec/qsvdec.c ##
> >> @@
> >> #include "hwconfig.h"
> >
> >
> > Is there any comment on this patchset ? If not, I'd like to merge
> it to make QSV
> > decoders works with SEI info.
> >
> > Thanks
> > Haihao
> >
>
> This patchset has several issues, namely:
> 1. It tries to share the functions that are used for processing
> user/SEI
> data as they are, even the parts that are not intended to be used by
> QSV
> (like the picture structure stuff for H.264 or tmpgexs in case of
> MPEG-1/2).
> 2. It tries to keep the functions where they are, leading to the
> insanely long Makefile line in patch 6/6 (which I believe to be still
> incomplete: mpeg12dec.o pulls in mpegvideo.o mpegvideo_dec.o (which
> in
> turn pull in lots of dsp stuff) and where is h264dsp.o? (it seems
> like
> there is a reliance on the H.264 parser for this)). This is the
> opposite
> of modularity.
> 3. It just puts a huge Mpeg1Context in the QSVContext, although only
> a
> miniscule part of it is actually used. One should use a small context
> of
> its own instead.
> 4. It does not take into account that buffers need to be padded to be
> usable by the GetBit-API.
Hi Andreas,
thanks for pointing out (4), in fact I wasn't aware of this.
I agree to your other points (1-3). Not that I wouldn't have been
aware of those implications, I've just been afraid that larger refactorings
could have minimized acceptance.
> (I have made an attempt to factor out the common parts of H.264 and
> H.265 SEI handling, which should make this here much easier.)
Your patchset would in fact be very helpful and allow me to provide
a much better and focused revision.
Though, it is still pending at this time - are you planning to push it?
Thanks,
softworkz
More information about the ffmpeg-devel
mailing list