[FFmpeg-devel] configure: fix module dependencies on zlib

Submitted by James Almer on Nov. 19, 2017, 7 p.m.

Details

Message ID 20171119190011.3624-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Nov. 19, 2017, 7 p.m.
select should not be used with external libraries. It's mean to soft
enable internal modules/features.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 configure | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

Comments

Michael Niedermayer Nov. 19, 2017, 8:33 p.m.
On Sun, Nov 19, 2017 at 04:00:11PM -0300, James Almer wrote:
> select should not be used with external libraries. It's mean to soft
> enable internal modules/features.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  configure | 48 ++++++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 22 deletions(-)

didnt test this but LGTM

thx

[...]
James Almer Nov. 19, 2017, 9:24 p.m.
On 11/19/2017 5:33 PM, Michael Niedermayer wrote:
> On Sun, Nov 19, 2017 at 04:00:11PM -0300, James Almer wrote:
>> select should not be used with external libraries. It's mean to soft
>> enable internal modules/features.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  configure | 48 ++++++++++++++++++++++++++----------------------
>>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> didnt test this but LGTM
> 
> thx

Pushed, thanks.
Michael Niedermayer Nov. 20, 2017, 5:34 p.m.
On Sun, Nov 19, 2017 at 04:00:11PM -0300, James Almer wrote:
> select should not be used with external libraries. It's mean to soft
> enable internal modules/features.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  configure | 48 ++++++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 22 deletions(-)

btw, is all the *suggest= zlib stuff correct ?

at least in the past configure could not handle a mix of
dependancies which turned targets on and which turned sources off
as the order in which they where executed resilted in different
outcomes


[...]
James Almer Nov. 20, 2017, 5:58 p.m.
On 11/20/2017 2:34 PM, Michael Niedermayer wrote:
> On Sun, Nov 19, 2017 at 04:00:11PM -0300, James Almer wrote:
>> select should not be used with external libraries. It's mean to soft
>> enable internal modules/features.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  configure | 48 ++++++++++++++++++++++++++----------------------
>>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> btw, is all the *suggest= zlib stuff correct ?
> 
> at least in the past configure could not handle a mix of
> dependancies which turned targets on and which turned sources off
> as the order in which they where executed resilted in different
> outcomes

'foo_suggest="bar"' nowadays is basically the same as doing 'enabled foo
&& enable_weak bar && enabled bar && append foo_extralibs
$bar_extralibs'. It does not disable foo under any circumstances.

enable_weak can't enable what was hard disabled, and zlib being
autodetected will be either hard enabled or hard disabled by the
automated check or by a command line option by the time dependency
checks are made.
Michael Niedermayer Nov. 20, 2017, 7:25 p.m.
On Mon, Nov 20, 2017 at 02:58:58PM -0300, James Almer wrote:
> On 11/20/2017 2:34 PM, Michael Niedermayer wrote:
> > On Sun, Nov 19, 2017 at 04:00:11PM -0300, James Almer wrote:
> >> select should not be used with external libraries. It's mean to soft
> >> enable internal modules/features.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>  configure | 48 ++++++++++++++++++++++++++----------------------
> >>  1 file changed, 26 insertions(+), 22 deletions(-)
> > 
> > btw, is all the *suggest= zlib stuff correct ?
> > 
> > at least in the past configure could not handle a mix of
> > dependancies which turned targets on and which turned sources off
> > as the order in which they where executed resilted in different
> > outcomes
> 
> 'foo_suggest="bar"' nowadays is basically the same as doing 'enabled foo
> && enable_weak bar && enabled bar && append foo_extralibs
> $bar_extralibs'. It does not disable foo under any circumstances.
> 
> enable_weak can't enable what was hard disabled, and zlib being
> autodetected will be either hard enabled or hard disabled by the
> automated check or by a command line option by the time dependency
> checks are made.

IIRC the issue i remember was:

'foo_suggest="bar"' or 'foo_select="bar"'
AND
'dot_deps="bar"'

had in the past unpredictable behavior because if foo_suggest/select is
evaluated first dot will be enabled and if dot_deps is evaluated first
dot would stay disabled.

That is the dependency solver would produce different results when
one would not expect it. On top of that the dependency solver recursivly
evaluted things making it a tad bit unpredictable what would get
evaluated first.

I remember thinking back then that all dependancies should be clear
one way, eiter only deps or only selects/suggest. (which was the case
back then for nearly all things as i rememer checking)

But its a while ago that i looked at this so i might misremember a detail
and there where various changes since then in configure.

but after a bit of looking now, i found my grep command i used to check
for this issue:

egrep '(suggest|select)=' configure | sed 's/.*=//;s/"//g' | tr ' ' '\n' | sort | uniq > select.tmp
egrep '(deps|deps_any)=' configure | sed 's/.*=//;s/"//g' | tr ' ' '\n' | sort | uniq >dep.tmp
cat select.tmp dep.tmp |sort | uniq -d

[...]
James Almer Nov. 21, 2017, 12:01 a.m.
On 11/20/2017 4:25 PM, Michael Niedermayer wrote:
> On Mon, Nov 20, 2017 at 02:58:58PM -0300, James Almer wrote:
>> On 11/20/2017 2:34 PM, Michael Niedermayer wrote:
>>> On Sun, Nov 19, 2017 at 04:00:11PM -0300, James Almer wrote:
>>>> select should not be used with external libraries. It's mean to soft
>>>> enable internal modules/features.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>  configure | 48 ++++++++++++++++++++++++++----------------------
>>>>  1 file changed, 26 insertions(+), 22 deletions(-)
>>>
>>> btw, is all the *suggest= zlib stuff correct ?
>>>
>>> at least in the past configure could not handle a mix of
>>> dependancies which turned targets on and which turned sources off
>>> as the order in which they where executed resilted in different
>>> outcomes
>>
>> 'foo_suggest="bar"' nowadays is basically the same as doing 'enabled foo
>> && enable_weak bar && enabled bar && append foo_extralibs
>> $bar_extralibs'. It does not disable foo under any circumstances.
>>
>> enable_weak can't enable what was hard disabled, and zlib being
>> autodetected will be either hard enabled or hard disabled by the
>> automated check or by a command line option by the time dependency
>> checks are made.
> 
> IIRC the issue i remember was:
> 
> 'foo_suggest="bar"' or 'foo_select="bar"'
> AND
> 'dot_deps="bar"'
> 
> had in the past unpredictable behavior because if foo_suggest/select is
> evaluated first dot will be enabled and if dot_deps is evaluated first
> dot would stay disabled.

That sounds like it would only be possible with _select, and I'm not
even sure if even then at all.
_deps of course doesn't try to enable anything, and _suggest can't
enable what was hard disabled, and external libraries (autodetected or
not) get hard disabled long before dependencies are resolved.

As long as bar was hard disabled by the time dependencies are resolved,
it shouldn't matter which module gets checked first in your scenario, if
dot or foo.

> 
> That is the dependency solver would produce different results when
> one would not expect it. On top of that the dependency solver recursivly
> evaluted things making it a tad bit unpredictable what would get
> evaluated first.
> 
> I remember thinking back then that all dependancies should be clear
> one way, eiter only deps or only selects/suggest. (which was the case
> back then for nearly all things as i rememer checking)
> 
> But its a while ago that i looked at this so i might misremember a detail
> and there where various changes since then in configure.

My bet is that yeah, so many changes have been made to configure that by
now such scenarios are probably not a problem anymore.

> 
> but after a bit of looking now, i found my grep command i used to check
> for this issue:
> 
> egrep '(suggest|select)=' configure | sed 's/.*=//;s/"//g' | tr ' ' '\n' | sort | uniq > select.tmp
> egrep '(deps|deps_any)=' configure | sed 's/.*=//;s/"//g' | tr ' ' '\n' | sort | uniq >dep.tmp
> cat select.tmp dep.tmp |sort | uniq -d
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Patch hide | download patch | download mbox

diff --git a/configure b/configure
index 8b7b7e164b..87a40e740b 100755
--- a/configure
+++ b/configure
@@ -2437,8 +2437,9 @@  amrwb_decoder_select="lsp"
 amv_decoder_select="sp5x_decoder exif"
 amv_encoder_select="aandcttables jpegtables mpegvideoenc"
 ape_decoder_select="bswapdsp llauddsp"
-apng_decoder_select="zlib"
-apng_encoder_select="llvidencdsp zlib"
+apng_decoder_deps="zlib"
+apng_encoder_deps="zlib"
+apng_encoder_select="llvidencdsp"
 asv1_decoder_select="blockdsp bswapdsp idctdsp"
 asv1_encoder_select="bswapdsp fdctdsp pixblockdsp"
 asv2_decoder_select="blockdsp bswapdsp idctdsp"
@@ -2465,14 +2466,14 @@  dnxhd_encoder_select="aandcttables blockdsp fdctdsp idctdsp mpegvideoenc pixbloc
 dolby_e_decoder_select="mdct"
 dvvideo_decoder_select="dvprofile idctdsp"
 dvvideo_encoder_select="dvprofile fdctdsp me_cmp pixblockdsp"
-dxa_decoder_select="zlib"
+dxa_decoder_deps="zlib"
 dxv_decoder_select="lzf texturedsp"
 eac3_decoder_select="ac3_decoder"
 eac3_encoder_select="ac3_encoder"
 eamad_decoder_select="aandcttables blockdsp bswapdsp idctdsp mpegvideo"
 eatgq_decoder_select="aandcttables"
 eatqi_decoder_select="aandcttables blockdsp bswapdsp idctdsp"
-exr_decoder_select="zlib"
+exr_decoder_deps="zlib"
 ffv1_decoder_select="rangecoder"
 ffv1_encoder_select="rangecoder"
 ffvhuff_decoder_select="huffyuv_decoder"
@@ -2480,15 +2481,16 @@  ffvhuff_encoder_select="huffyuv_encoder"
 fic_decoder_select="golomb"
 flac_decoder_select="flacdsp"
 flac_encoder_select="bswapdsp flacdsp lpc"
-flashsv2_decoder_select="zlib"
-flashsv2_encoder_select="zlib"
-flashsv_decoder_select="zlib"
-flashsv_encoder_select="zlib"
+flashsv2_decoder_deps="zlib"
+flashsv2_encoder_deps="zlib"
+flashsv_decoder_deps="zlib"
+flashsv_encoder_deps="zlib"
 flv_decoder_select="h263_decoder"
 flv_encoder_select="h263_encoder"
 fourxm_decoder_select="blockdsp bswapdsp"
 fraps_decoder_select="bswapdsp huffman"
-g2m_decoder_select="blockdsp idctdsp jpegtables zlib"
+g2m_decoder_deps="zlib"
+g2m_decoder_select="blockdsp idctdsp jpegtables"
 g729_decoder_select="audiodsp"
 h261_decoder_select="mpegvideo"
 h261_encoder_select="aandcttables mpegvideoenc"
@@ -2546,7 +2548,7 @@  mpeg2video_encoder_select="aandcttables mpegvideoenc h263dsp"
 mpeg4_decoder_select="h263_decoder mpeg4video_parser"
 mpeg4_encoder_select="h263_encoder"
 msa1_decoder_select="mss34dsp"
-mscc_decoder_select="zlib"
+mscc_decoder_deps="zlib"
 msmpeg4v1_decoder_select="h263_decoder"
 msmpeg4v2_decoder_select="h263_decoder"
 msmpeg4v2_encoder_select="h263_encoder"
@@ -2562,8 +2564,9 @@  on2avc_decoder_select="mdct"
 opus_decoder_deps="swresample"
 opus_decoder_select="mdct15"
 opus_encoder_select="audio_frame_queue mdct15"
-png_decoder_select="zlib"
-png_encoder_select="llvidencdsp zlib"
+png_decoder_deps="zlib"
+png_encoder_deps="zlib"
+png_encoder_select="llvidencdsp"
 prores_decoder_select="blockdsp idctdsp"
 prores_encoder_select="fdctdsp"
 qcelp_decoder_select="lsp"
@@ -2572,7 +2575,7 @@  ra_144_decoder_select="audiodsp"
 ra_144_encoder_select="audio_frame_queue lpc audiodsp"
 ralf_decoder_select="golomb"
 rawvideo_decoder_select="bswapdsp"
-rscc_decoder_select="zlib"
+rscc_decoder_deps="zlib"
 rtjpeg_decoder_select="me_cmp"
 rv10_decoder_select="h263_decoder"
 rv10_encoder_select="h263_encoder"
@@ -2580,7 +2583,7 @@  rv20_decoder_select="h263_decoder"
 rv20_encoder_select="h263_encoder"
 rv30_decoder_select="golomb h264pred h264qpel mpegvideo rv34dsp"
 rv40_decoder_select="golomb h264pred h264qpel mpegvideo rv34dsp"
-screenpresso_decoder_select="zlib"
+screenpresso_decoder_deps="zlib"
 shorten_decoder_select="bswapdsp"
 sipr_decoder_select="lsp"
 snow_decoder_select="dwt h264qpel hpeldsp me_cmp rangecoder videodsp"
@@ -2589,13 +2592,14 @@  sonic_decoder_select="golomb rangecoder"
 sonic_encoder_select="golomb rangecoder"
 sonic_ls_encoder_select="golomb rangecoder"
 sp5x_decoder_select="mjpeg_decoder"
-srgc_decoder_select="zlib"
+srgc_decoder_deps="zlib"
 svq1_decoder_select="hpeldsp"
 svq1_encoder_select="aandcttables hpeldsp me_cmp mpegvideoenc"
 svq3_decoder_select="golomb h264dsp h264parse h264pred hpeldsp tpeldsp videodsp"
 svq3_decoder_suggest="zlib"
 tak_decoder_select="audiodsp"
-tdsc_decoder_select="zlib mjpeg_decoder"
+tdsc_decoder_deps="zlib"
+tdsc_decoder_select="mjpeg_decoder"
 theora_decoder_select="vp3_decoder"
 thp_decoder_select="mjpeg_decoder"
 tiff_decoder_suggest="zlib lzma"
@@ -2604,7 +2608,7 @@  truehd_decoder_select="mlp_parser"
 truehd_encoder_select="lpc"
 truemotion2_decoder_select="bswapdsp"
 truespeech_decoder_select="bswapdsp"
-tscc_decoder_select="zlib"
+tscc_decoder_deps="zlib"
 twinvq_decoder_select="mdct lsp sinewin"
 txd_decoder_select="texturedsp"
 utvideo_decoder_select="bswapdsp llviddsp"
@@ -2638,11 +2642,11 @@  wmv3_decoder_select="vc1_decoder"
 wmv3image_decoder_select="wmv3_decoder"
 xma1_decoder_select="wmapro_decoder"
 xma2_decoder_select="wmapro_decoder"
-zerocodec_decoder_select="zlib"
-zlib_decoder_select="zlib"
-zlib_encoder_select="zlib"
-zmbv_decoder_select="zlib"
-zmbv_encoder_select="zlib"
+zerocodec_decoder_deps="zlib"
+zlib_decoder_deps="zlib"
+zlib_encoder_deps="zlib"
+zmbv_decoder_deps="zlib"
+zmbv_encoder_deps="zlib"
 
 # hardware accelerators
 crystalhd_deps="libcrystalhd_libcrystalhd_if_h"