[FFmpeg-devel] [PATCH 1/6] qsv: add ${includedir}/mfx to the search path for old versions of libmfx

[Xiang, Haihao]

On Tue, 2020-09-29 at 16:21 +0100, Mark Thompson wrote:
> On 16/09/2020 07:44, Haihao Xiang wrote:
> > ${includedir}/mfx has been added to Cflags in libmfx.pc for the current
> > libmfx. We may add ${includedir}/mfx to the search path for olda
> > versions of libmfx so that we may include foo.h instead of mfx/foo.h
> > 
> > After applying this change, we won't need to change #include to include
> > the right header files when the mfx header files are installed to a new
> > directory for a new version of libmfx. E.g. for libmfx 2.0 (oneVPL,
> > https://github.com/oneapi-src/oneVPL), the header files will be
> > installed in vpl directory
> > 
> > If your libmfx comes without pkg-config support, this patch requires a
> > small change to your environment setting (e.g. adding
> > /opt/intel/mediasdk/include/mfx instead of /opt/intel/mediasdk/include
> > to CFLAGS) so that the build can find the headers
> > ---
> >   configure                        | 15 ++++++++++++---
> >   fftools/ffmpeg_qsv.c             |  2 +-
> >   libavcodec/qsv.c                 |  8 ++++----
> >   libavcodec/qsv.h                 |  2 +-
> >   libavcodec/qsv_internal.h        |  2 +-
> >   libavcodec/qsvdec.c              |  2 +-
> >   libavcodec/qsvdec.h              |  2 +-
> >   libavcodec/qsvdec_h2645.c        |  2 +-
> >   libavcodec/qsvdec_other.c        |  2 +-
> >   libavcodec/qsvenc.c              |  2 +-
> >   libavcodec/qsvenc.h              |  2 +-
> >   libavcodec/qsvenc_h264.c         |  2 +-
> >   libavcodec/qsvenc_hevc.c         |  2 +-
> >   libavcodec/qsvenc_jpeg.c         |  2 +-
> >   libavcodec/qsvenc_mpeg2.c        |  2 +-
> >   libavcodec/qsvenc_vp9.c          |  2 +-
> >   libavfilter/qsvvpp.h             |  2 +-
> >   libavfilter/vf_deinterlace_qsv.c |  2 +-
> >   libavfilter/vf_scale_qsv.c       |  2 +-
> >   libavutil/hwcontext_opencl.c     |  2 +-
> >   libavutil/hwcontext_qsv.c        |  2 +-
> >   libavutil/hwcontext_qsv.h        |  2 +-
> >   22 files changed, 36 insertions(+), 27 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 5d68695192..767bc4ca69 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1458,6 +1458,14 @@ check_pkg_config(){
> >           eval add_cflags \$${name}_cflags
> >   }
> >   
> > +append_subdir_to_pkg_includedir(){
> > +    log append_subdir_to_pkg_includedir "$@"
> > +    name="$1"
> > +    subdir="$2"
> > +    incdir=$($pkg_config --variable=includedir $name)
> > +    add_cflags -I$incdir/$subdir
> > +}
> 
> Adding this hack as a separate function makes it look like other people would
> not be insane to use it, which is probably bad.

I was thinking that this function might be used in future if someone want to
append a subdir to includedir. 

> 
> > +
> >   test_exec(){
> >       test_ld "cc" "$@" && { enabled cross_compile || $TMPE >> $logfile
> > 2>&1; }
> >   }
> > @@ -6355,10 +6363,11 @@ enabled liblensfun        && require_pkg_config
> > liblensfun lensfun lensfun.h lf_
> >   # Media SDK or Intel Media Server Studio, these don't come with
> >   # pkg-config support.  Instead, users should make sure that the build
> >   # can find the libraries and headers through other means.
> > -enabled libmfx            && { check_pkg_config libmfx libmfx
> > "mfx/mfxvideo.h" MFXInit ||
> > -                               { require libmfx "mfx/mfxvideo.h" MFXInit "-
> > llibmfx $advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
> > +enabled libmfx            && { check_pkg_config libmfx libmfx "mfxvideo.h"
> > MFXInit ||
> > +                               { check_pkg_config libmfx libmfx
> > "mfx/mfxvideo.h" MFXInit && append_subdir_to_pkg_includedir libmfx mfx; } ||
> > +                               { require libmfx "mfxvideo.h" MFXInit "-
> > llibmfx $advapi32_extralibs" && warn "using libmfx without pkg-config"; } }
> 
> Checks for different ways of installing the same thing are very frowned
> upon.  Who actually breaks if you convert completely to the new style?
> 
> (Also consider the comment immediately above.  Is that still true?)

If I remembered correctly, some old versions of MSDK have the following settings
in libmfx.pc:

includedir=/usr/include
Cflags: -I${includedir}

But the header files are installed under /usr/include/mfx.

The old style works because 'mfx/foo.h' is included in source files, but for the
new style, it doesn't work if we don't add -I/usr/include/mfx to Cflags.

Thanks
Haihao

> 
> >   if enabled libmfx; then
> > -   check_cc MFX_CODEC_VP9 "mfx/mfxvp9.h mfx/mfxstructures.h"
> > "MFX_CODEC_VP9"
> > +   check_cc MFX_CODEC_VP9 "mfxvp9.h mfxstructures.h" "MFX_CODEC_VP9"
> >   fi
> >   
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".