<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 26/02/2020 15:54, Thomas Guillem
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:2829960a-9476-4f79-987b-53e3bb941716@www.fastmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <title></title>
      <style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style>
      <div><br>
      </div>
      <div><br>
      </div>
      <div>On Wed, Feb 26, 2020, at 15:34, Quentin Chateau wrote:<br>
      </div>
      <blockquote type="cite" id="qt">
        <p>Thanks for the hindsight, I dug further:<br>
        </p>
        <p>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...<br>
        </p>
        <p>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)<br>
        </p>
        <p>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:<br>
        </p>
        <ol>
          <li>Fix vlc_cond_timedwait so it will check for cancellation
            if the deadline is in the past<br>
          </li>
          <li>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<br>
          </li>
        </ol>
      </blockquote>
      <div><br>
      </div>
      <div>Or 3. Investigate why deadline is in the past.<br>
      </div>
    </blockquote>
    <p>That I can tell: I have a video filter that happens to fail on
      the last frame each video. <br>
    </p>
    <p>Everything goes as normal until the call to
      vout_ConvertForDisplay in ThreadDisplayRenderPicture, which
      returns NULL (result of my failing video filter) and causes an
      early return.</p>
    <pre>    todisplay = vout_ConvertForDisplay(vd, todisplay);
    if (todisplay == NULL) {
        vlc_mutex_unlock(&sys->display_lock);
        if (subpic != NULL)
            subpicture_Delete(subpic);
        return VLC_EGENERIC;
    }</pre>
    <p>This bypasses the update of sys->displayed.date that would
      have happened 50 lines below.<br>
    </p>
    <p>Because of that, sys->displayed.date never changes, in turn
      causing the deadtime variable to keep the same value (updated in
      ThreadDisplayPicture)<br>
    </p>
    <p><tt>    vlc_tick_t date_refresh = VLC_TICK_INVALID;</tt><tt><br>
      </tt><tt>    if (sys->displayed.date != VLC_TICK_INVALID) {</tt><tt><br>
      </tt><tt>        date_refresh = sys->displayed.date +
        VOUT_REDISPLAY_DELAY - render_delay;</tt><tt><br>
      </tt><tt>        refresh = date_refresh <= system_now;</tt><tt><br>
      </tt><tt>    }</tt><tt><br>
      </tt><tt>    bool force_refresh = !drop_next_frame &&
        refresh;</tt><tt><br>
      </tt><tt><br>
      </tt><tt>    if (!frame_by_frame) {</tt><tt><br>
      </tt><tt>        if (date_refresh != VLC_TICK_INVALID)</tt><tt><br>
      </tt><tt>            *deadline = date_refresh;</tt><tt><br>
      </tt><tt>        if (date_next != VLC_TICK_INVALID &&
        date_next < *deadline)</tt><tt><br>
      </tt><tt>            *deadline = date_next;</tt><tt><br>
      </tt><tt>    }</tt></p>
    <p>As time goes on, the value can get "in the past" before the
      thread is cancelled. From there the thread is never cancelled
      because of the behavior of pthread_cond_timedwait.</p>
    <blockquote type="cite"
      cite="mid:2829960a-9476-4f79-987b-53e3bb941716@www.fastmail.com">
      <div><br>
      </div>
      <div>Rémi, any preference ?<br>
      </div>
      <div><br>
      </div>
      <blockquote type="cite" id="qt">
        <p><br>
        </p>
        <div class="qt-moz-cite-prefix">On 26/02/2020 11:11, Thomas
          Guillem wrote:<br>
        </div>
        <blockquote
          cite="mid:2d816109-1f97-4b3e-8ffa-4b02394ca0e7@www.fastmail.com"
          type="cite">
          <pre class="qt-moz-quote-pre" wrap="">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, <a href="mailto:quentin.chateau@deepskycorp.com" class="qt-moz-txt-link-abbreviated" moz-do-not-send="true">quentin.chateau@deepskycorp.com</a> wrote:

</pre>
          <blockquote type="cite">
            <pre class="qt-moz-quote-pre" wrap="">From: Quentin Chateau <a href="mailto:quentin.chateau@deepskycorp.com" class="qt-moz-txt-link-rfc2396E" moz-do-not-send="true"><quentin.chateau@deepskycorp.com></a>

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:
<a href="https://mailman.videolan.org/listinfo/vlc-devel" class="qt-moz-txt-link-freetext" moz-do-not-send="true">https://mailman.videolan.org/listinfo/vlc-devel</a>

</pre>
          </blockquote>
          <pre class="qt-moz-quote-pre" wrap="">_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a href="https://mailman.videolan.org/listinfo/vlc-devel" class="qt-moz-txt-link-freetext" moz-do-not-send="true">https://mailman.videolan.org/listinfo/vlc-devel</a>
</pre>
        </blockquote>
        <div>_______________________________________________<br>
        </div>
        <div>vlc-devel mailing list<br>
        </div>
        <div>To unsubscribe or modify your subscription options:<br>
        </div>
        <div><a class="moz-txt-link-freetext" href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
        </div>
      </blockquote>
      <div><br>
      </div>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a class="moz-txt-link-freetext" href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre>
    </blockquote>
  </body>
</html>