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

[David Conrad]

On Apr 19, 2010, at 9:26 AM, M?ns Rullg?rd wrote:

> David Conrad <lessen42 at gmail.com> writes:
> 
>> 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.
> 
> Missed that.  Anyway, linking to pthreads unnecessarily seems silly.
> Past policy has always been to not enable things like this without
> being explicitly requested.  There's also still the question of other
> threading libs.  Why should we auto-enable only one?

We link to bzlib automatically even though it's only used by the matroska demuxer, and probably not a single file in the wild requires it. Contrast that to how nearly all new x86 chips have at least two virtual cores and would benefit from multithreading.

As for auto-enabling other libs, I'm not really saying we shouldn't, but since configure doesn't have any existing checks for them I'm not writing them when I don't have a dev system with them.

> Seriously, this is getting uglier each time.  What exactly are you
> trying to achieve?

Ideally: 

./configure
ffmpeg automatically uses a thread per core