On Tue, 16 Aug 2016 14:46:23 +0200 Fabien DESSENNE <fabien.desse...@st.com> wrote:
> 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? Hi, I didn't verify your reasoning, but it does sound plausible, yes. You might add a new 'post_frame_signal' perhaps for your purposes, or maybe something DRM-backend specific because of reliance on GBM. I can also imagine a reason why you would actually want to be one frame late: might calling into VAAPI cause a compositor stall? If VAAPI was blocking somehow, being a frame late allows you avoid blocking for the client and composite rendering. It's sub-optimal, but I am not familiar with VAAPI. If it doesn't have that issue, then adding a new hook sounds fine. Thanks, pq
pgpLmvdJohQvb.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel