[FFmpeg-devel] [PATCH 1/2] Check whether any decoders/encoders/etc are enabled after checking dependencies

[Måns Rullgård]

David Conrad <lessen42 at gmail.com> writes:

> On Apr 19, 2010, at 7:45 AM, M?ns Rullg?rd wrote:
>
>> David Conrad <lessen42 at gmail.com> writes:
>> 
>>> On Apr 19, 2010, at 7:15 AM, M?ns Rullg?rd wrote:
>>> 
>>>> David Conrad <lessen42 at gmail.com> writes:
>>>> 
>>>>> This will be needed to enable threads if pthreads is suggested
>>>>> ---
>>>>> configure |   26 +++++++++++++-------------
>>>>> 1 files changed, 13 insertions(+), 13 deletions(-)
>>>>> 
>>>>> diff --git a/configure b/configure
>>>>> index 25e8cef..9b9ae51 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -2795,19 +2795,6 @@ fi
>>>>> # Find out if the .align argument is a power of two or not.
>>>>> check_asm asmalign_pot '".align 3"'
>>>>> 
>>>>> -enabled_any $DECODER_LIST      && enable decoders
>>>>> -enabled_any $ENCODER_LIST      && enable encoders
>>>>> -enabled_any $HWACCEL_LIST      && enable hwaccels
>>>>> -enabled_any $BSF_LIST          && enable bsfs
>>>>> -enabled_any $DEMUXER_LIST      && enable demuxers
>>>>> -enabled_any $MUXER_LIST        && enable muxers
>>>>> -enabled_any $FILTER_LIST       && enable filters
>>>>> -enabled_any $INDEV_LIST        && enable indevs
>>>>> -enabled_any $OUTDEV_LIST       && enable outdevs
>>>>> -enabled_any $PROTOCOL_LIST     && enable protocols
>>>>> -
>>>>> -enabled_any $THREADS_LIST      && enable threads
>>>>> -
>>>>> check_deps $CONFIG_LIST       \
>>>>>           $CONFIG_EXTRA      \
>>>>>           $HAVE_LIST         \
>>>>> @@ -2823,6 +2810,19 @@ check_deps $CONFIG_LIST       \
>>>>>           $OUTDEV_LIST       \
>>>>>           $PROTOCOL_LIST     \
>>>>> 
>>>>> +enabled_any $DECODER_LIST      && enable decoders
>>>>> +enabled_any $ENCODER_LIST      && enable encoders
>>>>> +enabled_any $HWACCEL_LIST      && enable hwaccels
>>>>> +enabled_any $BSF_LIST          && enable bsfs
>>>>> +enabled_any $DEMUXER_LIST      && enable demuxers
>>>>> +enabled_any $MUXER_LIST        && enable muxers
>>>>> +enabled_any $FILTER_LIST       && enable filters
>>>>> +enabled_any $INDEV_LIST        && enable indevs
>>>>> +enabled_any $OUTDEV_LIST       && enable outdevs
>>>>> +enabled_any $PROTOCOL_LIST     && enable protocols
>>>>> +
>>>>> +enabled_any $THREADS_LIST      && enable threads
>>>>> +
>>>>> enabled asm || disable $ARCH_LIST $ARCH_EXT_LIST
>>>> 
>>>> Please explain why this is needed.
>>> 
>>> With autodetection, pthreads isn't explicitly enabled or disabled
>>> until the pthreads_if_any is evaluated by check_deps, so enabled_any
>>> doesn't think any threading methods are enabled and doesn't enable
>>> HAVE_THREADS, which is used.
>> 
>> I'm a bit sceptical to auto-enabling pthreads at all.  Also, why
>> auto-detect pthreads but none of the other threading libraries?
>
> 3 reasons:
> 1. pthreads is available for all major OSes. Even Haiku seems to have pthreads to some degree of functionality.
> 2. configure doesn't check anything for other threading libs
> 3. this shouldn't change anything if you use another threading lib by enabling it regardless of whether you have pthreads, nor if you don't have pthreads.

What if you have pthreads (as most systems do) but don't want it
enabled (e.g. only a single core)?  Going through pthreads adds
unnecessary overhead in the single-thread case.

I don't think this is a change we should make without due consideration.

>>> I moved the rest mainly for consistency (I can't find any CONFIG_*
>>> that would matter for them) and because it seems to me that they
>>> should be checked after check_deps has had a chance to do stuff.
>> 
>> Something might have a dependency on "encoders" etc, which would break
>> with this change.  OTOH, check_deps might disable all the things in
>> one of the lists too...
>
> The only dependencies on encoders, etc that I can find are at the C
> and makefile level (forgot about CONFIG_ENCODERS somehow.) And imo,
> to configure these should be implied from / a summary of everything
> else that's enabled.

Fixed properly.

-- 
M?ns Rullg?rd
mans at mansr.com