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

Måns Rullgård mans
Mon Apr 19 16:22:14 CEST 2010


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?

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



More information about the ffmpeg-devel mailing list