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

[David Conrad]

On Apr 19, 2010, at 8:58 AM, M?ns Rullg?rd wrote:

> 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.

pthread's avcodec_thread_init doesn't change execute() from the default if you only use one thread, so there's no run-time difference from not compiling it.

>>>> 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.

Thanks.