diff mbox

[FFmpeg-devel] configure: Allow users to disable all hwaccel libraries

Message ID 823a8c34-4768-67e9-28cb-eee0a2e009bb@gmail.com
State New
Headers show

Commit Message

Gyan Dec. 7, 2017, 9:31 a.m. UTC
On 12/7/2017 12:59 PM, Tobias Rapp wrote:
> 
> Why not add the HWACCEL_AUTODETECT_LIBRARY_LIST to --disable-hwaccels?

That is a better choice but there wasn't a bespoke case for 
'--disable-hwaccels', so originally ignored that possibility. Added it 
in attached (new) patch.

Either one - this or original patch - works for me.

Regards,
Gyan
From f5c02c73ad3560a2173611ec1e76a460cf70d944 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <gyandoshi@gmail.com>
Date: Thu, 7 Dec 2017 14:44:01 +0530
Subject: [PATCH] configure - extend disable_hwaccels to disable autodetected
 hwaccel libs

---
 configure | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Tobias Rapp Dec. 8, 2017, 11:26 a.m. UTC | #1
On 07.12.2017 10:31, Gyan Doshi wrote:
> On 12/7/2017 12:59 PM, Tobias Rapp wrote:
>>
>> Why not add the HWACCEL_AUTODETECT_LIBRARY_LIST to --disable-hwaccels?
> 
> That is a better choice but there wasn't a bespoke case for 
> '--disable-hwaccels', so originally ignored that possibility. Added it 
> in attached (new) patch.
> 
> Either one - this or original patch - works for me.

This one makes more sense to me as "disable all hwaccels" implicitly 
includes autodetected hwaccels, but that's just my personal opinion. I 
leave it to the maintainer of the file to decide what to push.

Regards,
Tobias
Gyan Dec. 8, 2017, 12:48 p.m. UTC | #2
On 12/8/2017 4:56 PM, Tobias Rapp wrote:

> includes autodetected hwaccels, but that's just my personal opinion. I 
> leave it to the maintainer of the file to decide what to push.

Speaking of which, who is the maintainer of configure? Neither the file 
nor MAINTAINERS has a name.

Regards,
Gyan
Gyan Dec. 10, 2017, noon UTC | #3
On 12/8/2017 6:18 PM, Gyan Doshi wrote:
> 
> On 12/8/2017 4:56 PM, Tobias Rapp wrote:
> 
>> includes autodetected hwaccels, but that's just my personal opinion. I 
>> leave it to the maintainer of the file to decide what to push.
> 
> Speaking of which, who is the maintainer of configure? Neither the file 
> nor MAINTAINERS has a name.
> 
> Regards,
> Gyan

Ping for the patch, and the question.
Carl Eugen Hoyos Dec. 10, 2017, 12:29 p.m. UTC | #4
2017-12-10 13:00 GMT+01:00 Gyan Doshi <gyandoshi@gmail.com>:

> Ping for the patch

Threaten to apply and wait a few days.

Carl Eugen
Mark Thompson Dec. 10, 2017, 5:14 p.m. UTC | #5
On 07/12/17 09:31, Gyan Doshi wrote:
> On 12/7/2017 12:59 PM, Tobias Rapp wrote:
>>
>> Why not add the HWACCEL_AUTODETECT_LIBRARY_LIST to --disable-hwaccels?
> 
> That is a better choice but there wasn't a bespoke case for '--disable-hwaccels', so originally ignored that possibility. Added it in attached (new) patch.
> 
> Either one - this or original patch - works for me.
> 
> From f5c02c73ad3560a2173611ec1e76a460cf70d944 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <gyandoshi@gmail.com>
> Date: Thu, 7 Dec 2017 14:44:01 +0530
> Subject: [PATCH] configure - extend disable_hwaccels to disable autodetected
>  hwaccel libs
> 
> ---
>  configure | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index d05388638d..77812759fd 100755
> --- a/configure
> +++ b/configure
> @@ -159,7 +159,7 @@ Individual component options:
>    --disable-decoders       disable all decoders
>    --disable-hwaccel=NAME   disable hwaccel NAME
>    --enable-hwaccel=NAME    enable hwaccel NAME
> -  --disable-hwaccels       disable all hwaccels
> +  --disable-hwaccels       disable all hwaccels and autodetected hwaccel libs
>    --disable-muxer=NAME     disable muxer NAME
>    --enable-muxer=NAME      enable muxer NAME
>    --disable-muxers         disable all muxers
> @@ -3630,6 +3630,9 @@ for opt do
>          --extra-libs=*)
>              add_extralibs $optval
>          ;;
> +        --disable-hwaccels)
> +            disable $HWACCEL_LIST $HWACCEL_AUTODETECT_LIBRARY_LIST
> +        ;;
>          --disable-devices)
>              disable $INDEV_LIST $OUTDEV_LIST
>          ;;
> -- 
> 2.11.1.windows.1

The --disable-foos options consistently disable all foos, I don't think it should magically do something else as well.  This is also confusing the internal concept of hwaccels (AVHWAccel) with other things providing access to hardware features (e.g. you are making it disable AMF and V4L2 M2M, which are not hwaccels).

Can you explain what you actually want to achieve here?  It sounds more like you want --disable-autodetect (hardware libraries are not the only ones which can give surprise dependencies).

- Mark
Gyan Dec. 10, 2017, 6:22 p.m. UTC | #6
On 12/10/2017 10:44 PM, Mark Thompson wrote:

> Can you explain what you actually want to achieve here?  It sounds more like you want --disable-autodetect (hardware libraries are not the only ones which can give surprise dependencies).

If the ffmpeg HW decoder/encoder wrappers are disabled, then associated 
libraries cuda, d3d11.. should also be disabled.

They are not because of autodetect. configure picks up 6 libs here. And 
since they are AD, users may not realize that they are included - I 
didn't; I thought disable-hwaccels did that - till I happened to check 
configure's result for another lib I was linking.

--disable-autodetect disables others besides the HW libs. The users may 
not know if they want/need these other libs, but if HW codecs are 
disabled, then those libs used exclusively by them, should not get linked.


Regards,
Gyan

P.S. Isn't AMF for providing support for AMD GPU decoder/encoders?
Mark Thompson Dec. 10, 2017, 6:36 p.m. UTC | #7
On 10/12/17 18:22, Gyan Doshi wrote:
> On 12/10/2017 10:44 PM, Mark Thompson wrote:
> 
>> Can you explain what you actually want to achieve here?  It sounds more like you want --disable-autodetect (hardware libraries are not the only ones which can give surprise dependencies).
> 
> If the ffmpeg HW decoder/encoder wrappers are disabled, then associated libraries cuda, d3d11.. should also be disabled.
> 
> They are not because of autodetect. configure picks up 6 libs here. And since they are AD, users may not realize that they are included - I didn't; I thought disable-hwaccels did that - till I happened to check configure's result for another lib I was linking.

--disable-hwaccels does not disable hardware codecs: it disables hwaccels only, decoders and encoders will still be present.

If you want to disable decoder and encoder wrappers as well then you will need to either use --disable-decoders --disable-encoders or disable them individually.

(And that still doesn't disable filtering or hwcontext generic support using these libraries, which can be used separately.)

> --disable-autodetect disables others besides the HW libs. The users may not know if they want/need these other libs, but if HW codecs are disabled, then those libs used exclusively by them, should not get linked.
Under what circumstances does a user want to disable autodetect for some subset of libraries while allowing it for others?  There can still be surprise dependencies, which is presumably the thing they want to avoid.

> P.S. Isn't AMF for providing support for AMD GPU decoder/encoders?

Just encoders, but yes.

- Mark
Gyan Dec. 10, 2017, 6:53 p.m. UTC | #8
On 12/11/2017 12:06 AM, Mark Thompson wrote:

> --disable-hwaccels does not disable hardware codecs: it disables hwaccels only, decoders and encoders will still be present.
> 
> If you want to disable decoder and encoder wrappers as well then you will need to either use --disable-decoders --disable-encoders or disable them individually.

Sounds unwieldy. All solutions seem to me to require creating new lists. 
Patch withdrawn.

> Under what circumstances does a user want to disable autodetect for some subset of libraries while allowing it for others?  There can still be surprise dependencies, which is presumably the thing they want to avoid.

Not surprise deps but deadweights. If nvdec wrapper isn't enabled then 
nvdec shouldn't be linked.  Not a concern for most libs which are 
manually enabled.

Thanks.
diff mbox

Patch

diff --git a/configure b/configure
index d05388638d..77812759fd 100755
--- a/configure
+++ b/configure
@@ -159,7 +159,7 @@  Individual component options:
   --disable-decoders       disable all decoders
   --disable-hwaccel=NAME   disable hwaccel NAME
   --enable-hwaccel=NAME    enable hwaccel NAME
-  --disable-hwaccels       disable all hwaccels
+  --disable-hwaccels       disable all hwaccels and autodetected hwaccel libs
   --disable-muxer=NAME     disable muxer NAME
   --enable-muxer=NAME      enable muxer NAME
   --disable-muxers         disable all muxers
@@ -3630,6 +3630,9 @@  for opt do
         --extra-libs=*)
             add_extralibs $optval
         ;;
+        --disable-hwaccels)
+            disable $HWACCEL_LIST $HWACCEL_AUTODETECT_LIBRARY_LIST
+        ;;
         --disable-devices)
             disable $INDEV_LIST $OUTDEV_LIST
         ;;