diff mbox

[FFmpeg-devel] Added require fallback for libmfx in the case that pkg-config cannot find libmfx

Message ID 26a73910-b0bb-a2d7-85fe-52339a25029a@aracnet.com
State Accepted
Commit 164e2773261451ef33c4616296ec5bebecff42af
Headers show

Commit Message

Aaron Levinson May 6, 2017, 1:32 a.m. UTC 
On 5/5/2017 4:50 AM, Michael Niedermayer wrote:
> On Fri, May 05, 2017 at 11:36:05AM +0200, Hendrik Leppkes wrote:
>> On Fri, May 5, 2017 at 9:57 AM, Clément Bœsch <u@pkh.me> wrote:
>>> On Fri, May 05, 2017 at 12:54:12AM -0700, Aaron Levinson wrote:
>>> [...]
>>>>> Back to your issue: you should fix the .pc in the upstream project, this
>>>>> is the correct fix.
>>>>
>>>> The "upstream project" in this case is the Intel Media SDK.  This is not an
>>>> open source project, and developers get it through Intel in the form of an
>>>> installation package on Windows.  So, there is no opportunity to "fix the
>>>> .pc"--there isn't one, nor is there any way to contribute one.
>>>
>>> OK so we have a common configure flag for supporting two different
>>> projects with different authors, licensing, etc?
>>>
>>> Maybe we should have 2 configure flags? Add --enable-libmfxintel or
>>> whatever?
>>>
>>
>> I think having separate flags would just be annoying. Lucas
>> redistribution is in no sense "official" in any way, and what if
>> someone else comes up with another re-package of the same thing?
> 
>> The key information here is, upstream libmfx does not actually have a
>> pkg-config file, that some re-distributions add one is nice and all,
>> and we can use that if present, but we should also be able to use the
>> upstream variant which does not come with one at all.
> 
> +1

Per some discussion on IRC, I've created a new patch that includes a detailed comment in configure for why require is being used.  I also altered the text for the patch.

Thanks,
Aaron Levinson

-----------------------------------------------------------------------

From 5d8fb32801accc06655c4fae700652958bc4350e Mon Sep 17 00:00:00 2001
From: Aaron Levinson <alevinsn@aracnet.com>
Date: Fri, 5 May 2017 18:16:03 -0700
Subject: [PATCH] configure: Added require alternative for libmfx to support
 alternate installation options

Purpose: Added require alternative for libmfx in the case that pkg-config
cannot find libmfx.  On Linux, most people likely get libmfx via
https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
well-known way to get libmfx is via the Intel Media SDK, which
provides a static build of libmfx.lib and also provides the source
code for building libmfx yourself.  If built this way, there are no
pkg-config files to be found.

Comments:

-- configure: Altered enabled libmfx step to use use_pkg_config()
   instead of require_pkg_config(), and, if use_pkg_config() fails, it
   falls back to require().  Also added explanatory comment.  Note
   that the reason that require() is passed -llibmfx as the last
   argument, instead of -lmfx, is the file name for the library
   produced from the Intel Media SDK starts with "libmfx".
   Apparently, the filename for the library produced via
   https://github.com/lu-zero/mfx_dispatch starts with "mfx".

Signed-off-by: Aaron Levinson <alevinsn@aracnet.com>
---
 configure | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer May 9, 2017, 1:37 a.m. UTC | #1 
On Fri, May 05, 2017 at 06:32:18PM -0700, Aaron Levinson wrote:
> On 5/5/2017 4:50 AM, Michael Niedermayer wrote:
> > On Fri, May 05, 2017 at 11:36:05AM +0200, Hendrik Leppkes wrote:
> >> On Fri, May 5, 2017 at 9:57 AM, Clément Bœsch <u@pkh.me> wrote:
> >>> On Fri, May 05, 2017 at 12:54:12AM -0700, Aaron Levinson wrote:
> >>> [...]
> >>>>> Back to your issue: you should fix the .pc in the upstream project, this
> >>>>> is the correct fix.
> >>>>
> >>>> The "upstream project" in this case is the Intel Media SDK.  This is not an
> >>>> open source project, and developers get it through Intel in the form of an
> >>>> installation package on Windows.  So, there is no opportunity to "fix the
> >>>> .pc"--there isn't one, nor is there any way to contribute one.
> >>>
> >>> OK so we have a common configure flag for supporting two different
> >>> projects with different authors, licensing, etc?
> >>>
> >>> Maybe we should have 2 configure flags? Add --enable-libmfxintel or
> >>> whatever?
> >>>
> >>
> >> I think having separate flags would just be annoying. Lucas
> >> redistribution is in no sense "official" in any way, and what if
> >> someone else comes up with another re-package of the same thing?
> > 
> >> The key information here is, upstream libmfx does not actually have a
> >> pkg-config file, that some re-distributions add one is nice and all,
> >> and we can use that if present, but we should also be able to use the
> >> upstream variant which does not come with one at all.
> > 
> > +1
> 
> Per some discussion on IRC, I've created a new patch that includes a detailed comment in configure for why require is being used.  I also altered the text for the patch.
> 
> Thanks,
> Aaron Levinson
> 
> -----------------------------------------------------------------------
> 
> From 5d8fb32801accc06655c4fae700652958bc4350e Mon Sep 17 00:00:00 2001
> From: Aaron Levinson <alevinsn@aracnet.com>
> Date: Fri, 5 May 2017 18:16:03 -0700
> Subject: [PATCH] configure: Added require alternative for libmfx to support
>  alternate installation options
> 
> Purpose: Added require alternative for libmfx in the case that pkg-config
> cannot find libmfx.  On Linux, most people likely get libmfx via
> https://github.com/lu-zero/mfx_dispatch , but on Windows, the most
> well-known way to get libmfx is via the Intel Media SDK, which
> provides a static build of libmfx.lib and also provides the source
> code for building libmfx yourself.  If built this way, there are no
> pkg-config files to be found.
> 
> Comments:
> 
> -- configure: Altered enabled libmfx step to use use_pkg_config()
>    instead of require_pkg_config(), and, if use_pkg_config() fails, it
>    falls back to require().  Also added explanatory comment.  Note
>    that the reason that require() is passed -llibmfx as the last
>    argument, instead of -lmfx, is the file name for the library
>    produced from the Intel Media SDK starts with "libmfx".
>    Apparently, the filename for the library produced via
>    https://github.com/lu-zero/mfx_dispatch starts with "mfx".
> 
> Signed-off-by: Aaron Levinson <alevinsn@aracnet.com>
> ---
>  configure | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

applied

thx

[...]
diff mbox

Patch

diff --git a/configure b/configure
index a008b41473..296bff9dc7 100755
--- a/configure
+++ b/configure
@@ -5788,7 +5788,14 @@  enabled libgsm            && { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
                                done || die "ERROR: libgsm not found"; }
 enabled libilbc           && require libilbc ilbc.h WebRtcIlbcfix_InitDecode -lilbc
 enabled libkvazaar        && require_pkg_config "kvazaar >= 0.8.1" kvazaar.h kvz_api_get
-enabled libmfx            && require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit
+# While it may appear that require is being used as a pkg-config
+# fallback for libmfx, it is actually being used to detect a different
+# installation route altogether.  If libmfx is installed via the Intel
+# 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            && { use_pkg_config libmfx "mfx/mfxvideo.h" MFXInit ||
+                               { require libmfx "mfx/mfxvideo.h" MFXInit -llibmfx && warn "using libmfx without pkg-config"; } }
 enabled libmodplug        && require_pkg_config libmodplug libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame        && require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame
 enabled libnut            && require libnut libnut.h nut_demuxer_init -lnut