[vlc-devel] [PATCH] video_output: explicit vlc_testcancel

[Quentin Chateau]

Thanks for the hindsight, I dug further:

vlc_cond_timedwait is called, and so is pthread_cond_timedwait. The 
issue happens when the deadline parameter is in the past: when this 
happens, pthread_cond_timedwait does not act as a cancellation point 
anymore...

In my case deadline happens to be in the past because 
ThreadDisplayPicture keeps failing (due to one of my video filters 
failing on the last frame)

I could not find this behavior in the pthread documentation, but it 
seems to be the case anyway. Do you think that we need to:

 1. Fix vlc_cond_timedwait so it will check for cancellation if the
    deadline is in the past
 2. Acknowledge the fact that vlc_cond_timedwait is not always a
    cancellation point and fix the issue higher in the stack, either in
    vout_control_Pop, or similarly to the patch I sent, directly in Thread


On 26/02/2020 11:11, Thomas Guillem wrote:
> Why is it not canceled from vlc_cond_timedwait() called from vout_control_Pop() ?
>
> Is ctrl->can_sleep false in your case ?
>
> Anyway, I would prefer a fix in vout_control_Pop() like this:
>
>      if (ctrl->cmd.i_size <= 0 && deadline != VLC_TICK_INVALID && ctrl->can_sleep) {
>          /* Spurious wakeups are perfectly fine */
>          ctrl->is_waiting = true;
>          vlc_cond_signal(&ctrl->wait_available);
>          vlc_cond_timedwait(&ctrl->wait_request, &ctrl->lock, deadline);
>          ctrl->is_waiting = false;
>      }
>      else
>          vlc_testcancel();
>
> On Tue, Feb 25, 2020, at 12:47, quentin.chateau at deepskycorp.com wrote:
>> From: Quentin Chateau <quentin.chateau at deepskycorp.com>
>>
>> When ThreadDisplayPicture does not return VLC_SUCCESS,
>> explicitely check for thread cancellation.
>>
>> There are cases where a video filter can fail
>> systematically without hitting any cancellation point
>> after the video output thread is cancelled, resulting
>> in an infinite loop if there is no explicit check.
>> ---
>>   src/video_output/video_output.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>> index 229b010a96..f7aac554e4 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -1703,6 +1703,7 @@ noreturn static void *Thread(void *object)
>>   
>>           if (wait)
>>           {
>> +            vlc_testcancel();
>>               const vlc_tick_t max_deadline = vlc_tick_now() +
>> VLC_TICK_FROM_MS(100);
>>               deadline = deadline == VLC_TICK_INVALID ? max_deadline :
>> __MIN(deadline, max_deadline);
>>           } else {
>> -- 
>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200226/b47a52da/attachment.html>