On Sun, 2022-05-01 at 03:10 +0000, Soft Works wrote: > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > Xiang, Haihao > > Sent: Sunday, May 1, 2022 4:16 AM > > To: an...@khirnov.net; ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH v7 05/10] qsv: build audio related > > code when MFX_VERSION < 2.0 > > > > On Sat, 2022-04-30 at 16:51 +0000, Soft Works wrote: > > > > -----Original Message----- > > > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > > > Xiang, Haihao > > > > Sent: Wednesday, April 6, 2022 5:38 AM > > > > To: an...@khirnov.net; ffmpeg-devel@ffmpeg.org > > > > Subject: Re: [FFmpeg-devel] [PATCH v7 05/10] qsv: build audio > > > > related > > > > code when MFX_VERSION < 2.0 > > > > > > > > On Tue, 2022-04-05 at 13:50 +0200, Anton Khirnov wrote: > > > > > Quoting Xiang, Haihao (2022-03-11 09:16:25) > > > > > > From: Haihao Xiang <haihao.xi...@intel.com> > > > > > > > > > > > > Audio isn't supported for MFX_VERSION >= 2.0[1][2]. This is in > > > > > > preparation for oneVPL support > > > > > > > > > > > > [1]: > > > > > > > > > > > > > > > > > > https://spec.oneapi.io/versions/latest/elements/oneVPL/source/VPL_inte > > > > l_media_sdk.html#msdk-full-name-feature-removals > > > > > > [2]: https://github.com/oneapi-src/oneVPL > > > > > > --- > > > > > > libavcodec/qsv.c | 5 +++++ > > > > > > libavfilter/qsvvpp.c | 6 ++++++ > > > > > > libavfilter/qsvvpp.h | 2 ++ > > > > > > 3 files changed, 13 insertions(+) > > > > > > > > > > Why not just remove this completely? > > > > > None of our QSV code does anything with audio. > > > > > > > > It was removed in an older version, however someone objected the > > > > removal of > > > > this. See > > > > > > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200819061023.44330 > > > > 4-2-haihao.xi...@intel.com/ > > > > > > I think this was a misunderstanding. I see not objection. One was > > > just asking "why" and the other one had missed the point that audio > > > has never been functional. > > > > Please find the comment below in the original thread > > > > "This seems like a generic translation from the library errors to FF > > error > > codes. So even if we'll never touch the audio functionality of it, I'd > > prefer > > to have that struct complete already" > > > > So my understanding was that the reviewer preferred to keep the audio > > stuff > > unchanged for libmfx. > > Hm, I hadn't see that here: > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200819061023.443304-2-haihao.xi...@intel.com/ > > Considering the text again: > > > "This seems like a generic translation from the library errors to FF > > error > > codes. So even if we'll never touch the audio functionality of it, I'd > > prefer > > to have that struct complete already" > > I understand the idea. Normally those lines wouldn't hurt. But now, that > we're facing some kind of "#ifdef hell" anyway, I think it would be much > better to minimize this as much as possible, and there's really no point > in translating audio error codes. > Also, the struct has never been really complete. Instead of retaining > unused audio error codes, we should better add those that are missing > (like -21, -22 and others) and relevant. > > @Thilo - can we get you warm with that? > > As an alternative, we could simply replace the two audio definitions with > plain integer values (MFX_ERR_INVALID_AUDIO_PARAM >> -19) > > > This is a small bit only, but before adding the oneVPL stuff, I think > we should consolidate the conditional stuff as much as possible. > > As discussed before, we also need to settle for a minimum libmfx SDK > version (compile-time, not runtime!). This will allow to drop quite > an amount of conditional code, and this cleanup should be done before > getting to oneVPL.
Sure, I'll submit a patch for it. > > Another thing that is a bit unfortunate is that we are duplicating this > error mapping struct in qsv.c and qsvvpp.c. > I don't mean that it should be linked as an external between avfilter > and avcodec, but it should come (be included) from a single file. We moved the static error table to a .h in the past however it resulted in link error when building FFmpeg with static libraries. Thanks Haihao _______________________________________________ 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".
