[vlc-devel] [PATCH 3/7] video_output: asset the display existence inside the display_lock

[Steve Lhomme]

On 2019-10-09 17:57, Thomas Guillem wrote:
> Not needed, you shoud add a documentation that says that is API is not thread safe but reentrant like decoder_updatevideoformat()

Same remark, it requires to know the whole code around this function to 
read 4 lines that would otherwise be totally coherent. It requires 
reading a whole lot more code for the sake of it (and reading 
multithread code in a 2000 lines file is anything but easy).

In other words it means: if you intend to touch this file, you have to 
understand all of it. What may seem straightforward is not.

If a comment is needed that's why a variable with a dedicated lock is 
read without that lock. What kind of multithreading good practice is that ?

> On Wed, Oct 9, 2019, at 17:36, Steve Lhomme wrote:
>> ---
>>   src/video_output/video_output.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/src/video_output/video_output.c
>> b/src/video_output/video_output.c
>> index df27a402b8b..e8ee9572188 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -1346,8 +1346,6 @@ void vout_ChangePause(vout_thread_t *vout, bool
>> is_paused, vlc_tick_t date)
>>   static void vout_FlushUnlocked(vout_thread_t *vout, bool below,
>>                                  vlc_tick_t date)
>>   {
>> -    vout_thread_sys_t *sys = vout->p;
>> -
>>       vout->p->step.timestamp = VLC_TICK_INVALID;
>>       vout->p->step.last      = VLC_TICK_INVALID;
>>   
>> @@ -1368,8 +1366,8 @@ static void vout_FlushUnlocked(vout_thread_t
>> *vout, bool below,
>>   
>>       picture_fifo_Flush(vout->p->decoder_fifo, date, below);
>>   
>> -    assert(sys->display != NULL);
>>       vlc_mutex_lock(&vout->p->display_lock);
>> +    assert(vout->p->display != NULL);
>>       vout_FilterFlush(vout->p->display);
>>       vlc_mutex_unlock(&vout->p->display_lock);
>>   
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>