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

[Alexander Strange]

On Apr 19, 2010, at 10:22 AM, M?ns Rullg?rd wrote:

> David Conrad <lessen42 at gmail.com> writes:
> 
>> 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.
> 
> So turn it off by default then.  If you can only find a single,
> obscure case supporting your suggestion, chances are that case is
> wrong and should be fixed.  I'm sick and tired of people using bad
> exceptions as justification for adding more broken things.
> 
> I'm not necessarily saying what you are looking to do here is wrong,
> but you'll need to find a better reason for doing 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.
> 
> The way you're heading will make it difficult for anyone to write
> those bits.
> 
>>> Seriously, this is getting uglier each time.  What exactly are you
>>> trying to achieve?
>> 
>> Ideally: 
>> 
>> ./configure
>> ffmpeg automatically uses a thread per core
> 
> Who said that's what the user wants?

What else would they want? It's not like anyone wants it to run slower.

Other reasons:
- if it's not enabled by default, it's not tested by default, and h264 slice threading has broken twice already recently
- I'm really tired of typing --enable-pthreads