[FFmpeg-devel] [PATCH v7 05/10] qsv: build audio related code when MFX_VERSION < 2.0

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Sunday, May 1, 2022 4:16 AM
> To: anton at khirnov.net; ffmpeg-devel at 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-bounces at ffmpeg.org> On Behalf Of
> > > Xiang, Haihao
> > > Sent: Wednesday, April 6, 2022 5:38 AM
> > > To: anton at khirnov.net; ffmpeg-devel at 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.xiang at 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.xiang at 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.xiang@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.

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.

Kind regards,
softworkz