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 <softwo...@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. (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.) - Andreas _______________________________________________ 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".
