[FFmpeg-devel] [PATCH 1/3] avutil/log: add AV_LOG_ASYNC

[wm4]

On Thu, 20 Mar 2014 14:07:05 +0530
anshul <anshul.ffmpeg at gmail.com> wrote:

> On 03/20/2014 11:54 AM, wm4 wrote:
> > On Thu, 20 Mar 2014 03:57:57 +0100
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> >
> >
> >> +/**
> >> + * Do not thread synchronization.
> >> + * This allows av_log() to be used from a signal handler but may lead to
> >> + * garbled output.
> >> + */
> >> +#define AV_LOG_ASYNC 0x1000
> >>   void av_log_set_flags(int arg);
> Is there any special reason for value 0x1000
> Why not 0x0002 or 0x0100?

I suspect this is because of ABI compatibility with Libav. If they add
a flag, FFmpeg couldn't be ABI compatible. (Nevermind that FFmpeg is
too different from Libav anyway as that ABI compatibility would be
usaful in practice.)

> As per my understanding ASYNC and SKIP_REPEATED both flag change 
> properties of log mechanism.
> so both should have similar numbering otherwise more properties related 
> flag would result in chaos.
> 
> Since your changes are related to deadlock condition when same thread 
> apply lock twice, there may be better
> approach if we check pthread_self().
> 
> Some comments from manual of pthread_mutex_lock
>         The mutex functions and the particular default settings of the 
> mutex attributes have been motivated by the desire to not preclude  
> fast,  inlined
>         implementations of mutex locking and unlocking.
> 
>         For  example, deadlocking on a double-lock is explicitly allowed 
> behavior in order to avoid requiring more overhead in the basic 
> mechanism than is
>         absolutely necessary. (More "friendly" mutexes that detect 
> deadlock or that allow multiple locking by the same thread are easily  
> constructed  by
>         the  user via the other mechanisms provided. For example, 
> pthread_self() can be used to record mutex ownership.) Implementations 
> might also choose
>         to provide such extended features as options via special mutex 
> attributes.
> > Just for ffmpeg.c? Please no...
> This would help other application too, it is just now used in ffmpeg.c 
> will take time to adapt this feature by other application.
> So yes

No. I'm afraid that broken things will start to use it from other
signal handlers too. Doing too much in signal handlers (even just
logging) is insane, and I think *printf() is not even on the list of
async signal safe functions.

Instead, the code should be cleaned up not to call av_log from a signal
handler.