[Bug 740803] waylandsink: Double-check the frame callback before dropping frames

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740803

--- Comment #10 from Praveen R Jadhav <praveen.j at samsung.com> ---
(In reply to George Kiagiadakis from comment #9)
> This patch will create issues in the streaming thread when the wayland
> server stops rendering the video window (because it may not be visible or
> maybe because the lock screen is on). It is perfectly valid for the server
> to do this and may take a long time (in case of the lock screen for example)
> for the frame callback to be called. In the meantime, the streaming thread
> *should* continue working at the same rate and drop those frames that are
> not going to be displayed anyway. If you start waiting an additional frame
> duration for every frame sync, then GstBaseSink will find that every one in
> two frames is delivered too late and will start firing warnings and QoS
> events.

I too accept that 1 frame duration is a bit longer and is a worst case
scenario. I have tested various scenarios with this patch, but din't find any
case where frames are dropped because of timeout.(Regarding wayland server
delay during screen lock, profiles I work on don't render video in this
scenario). About the streaming scenario, if the wayland server delays
intentionally, then QoS events will be triggered for sure. Reducing wait time
to a lesser duration will minimize render thread faltering.

> 
> (In reply to Praveen R Jadhav from comment #6)
> > This conditional wait operation will handle race conditions when render
> > requests are made from multiple paths.
> 
> I'm not sure how you mean that multiple requests can be made from multiple
> paths. If you mean expose() and show_frame() being called at the same time,
> I think the solution is to just remove the implementation of expose()
> completely. I implemented it a couple of years ago just to follow the
> pattern for all video sinks, but I haven't found any practical use for it in
> wayland.

While rendering 2 videos(local and remote) during video call, If user swaps
video screens, then set_render_rect() and expose() are called to update new
display regions. In this scenario, we can clearly observe background if
expose() request to render last frame is dropped. This patch ensures last frame
is updated forcefully and flicker issues are not observed.


@ Nicolas Dufresne: As mentioned above, 1 frame duration is the worst case
scenario. I tested using wl_display_roundtrip_queue()/wl_display_roundtrip().
They are blocking calls and render thread has to wait even though
frame_redraw_callback() is already returned from server. This delays even
further. Using g_cond is one approach reduces overall delay because of blocking
call in render thread. Using presentation time will not help for expose()
cases.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.