On 08/16/2016 10:18 AM, Pekka Paalanen wrote: > On Fri, 12 Aug 2016 14:11:40 +0200 > Fabien Dessenne <fabien.desse...@st.com> wrote: > >> Emit frame_signal at the end of the GL renderer processing, so the >> frame_signal clients are informed after the buffer is actually updated. >> >> Signed-off-by: Fabien Dessenne <fabien.desse...@st.com> >> --- >> libweston/gl-renderer.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c >> index d624453..f4d4a5e 100644 >> --- a/libweston/gl-renderer.c >> +++ b/libweston/gl-renderer.c >> @@ -1138,7 +1138,6 @@ gl_renderer_repaint_output(struct weston_output >> *output, >> draw_output_borders(output, border_damage); >> >> pixman_region32_copy(&output->previous_damage, output_damage); >> - wl_signal_emit(&output->frame_signal, output); >> >> if (gr->swap_buffers_with_damage) { >> pixman_region32_init(&buffer_damage); >> @@ -1185,6 +1184,7 @@ gl_renderer_repaint_output(struct weston_output >> *output, >> } >> >> go->border_status = BORDER_STATUS_CLEAN; >> + wl_signal_emit(&output->frame_signal, output); >> } >> >> static int > Hi, > > I think this will break screenshooting. > > Screenshooter relies on glReadPixels, but if you issue eglSwapBuffers > first, you no longer know what glReadPixels will return. Therefore the > signal must be emitted before the swapbuffers. Oh, yes, you are right, I missed this part. > You forgot to explain what problem this change solves, too. I am trying to improve the way to record the display since glReadPixels() is quite slow: although it works fine for a one frame capture, the performances are not acceptable for a continuous capture.
So I am prototyping something between screenshooter and VAAPI recorder but my problem is that my recorder is always one frame late. This issue was solved with my proposed patch, but I agree that it is not acceptable. Now I come to the conclusion that VAAPI recorder has probably the same 'one frame late' issue, which I unfortunately cannot verify, but reading the code let me think that there is something wrong there: drm_output_render_gl() calls: - output->base.compositor->renderer->repaint_output() - bo = gbm_surface_lock_front_buffer(output->gbm_surface) - output->next = drm_fb_get_from_bo(bo, b, output->gbm_format) Then, "output->current" is updated with the fresh "output->next" at the next page_flip_handler() call. And before that, VAAPI recorder captures "output->current" (see recorder_frame_notify) just after repaint_output() is called (triggered by frame_signal emit). But at that time, "output->current" is not updated yet, it still refers to the previous frame (it is updated at the next page_flip) Can you let me know your opinion about it? Fabien > I only see it breaking things. > > > Thanks, > pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel