Re: [Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
Hi Frediano, > > Il giorno ven 13 gen 2023 alle ore 04:08 Kasireddy, Vivek > ha scritto: > > > > Hi Frediano, > > > > > > > > Il giorno gio 12 gen 2023 alle ore 07:03 Kasireddy, Vivek > > > ha scritto: > > > > > > > > Hi Frediano, > > > > > > > > > > > > > > Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy > > > > > ha scritto: > > > > > > > > > > > > This async is triggered by the encoder indicating that the > > > > > > encoding process is completed. > > > > > > > > > > > > > > > > This description does not make much sense to me. > > > > > You are adding a public function which, I suppose, should be called by > > > > > Qemu but you are stating the encoder is calling it. > > > > > Unless Qemu is the encoder it does not make sense. > > > > [Kasireddy, Vivek] Sorry for the poor, misleading description. I > > > > originally > > > > had this patch split into two, one for each async function and forgot to > > > > update the commit message when I merged them. I'll update the > > > > commit message in v2 which would probably have the following desc: > > > > "This patch mainly adds two async functions: a public one and an > > > > internal one. The public function spice_qxl_dmabuf_encode_async is > > > > used by applications that would share their fd with the Spice server. > > > > > > I don't see a reason for the new API, it's just doing a combination of > > > spice_qxl_gl_scanout + spice_qxl_gl_draw_async. > > [Kasireddy, Vivek] I had considered reusing these functions for my use-case > > but decided against cluttering them. Anyway, do you think it is ok to add a > > new parameter or a flag to these public APIs to prevent them from doing > > qxl_state->send_message() because my use-case does not involve sending > > any messages (neither RedWorkerMessageGlDraw nor > > RedWorkerMessageGlScanout) to the client. > > > > On the other end you are cluttering public API, Qemu and user usage. > To use local connection you have to setup the VM in a different way > than remote connection so you would have either to ask the > administrator to reconfigure and restart the VM or having a possible > suboptimal (or not working) connection. Doing inside spice-server > won't require VM setup change (because spice-server can choose the way > of handling it). > About qxl_state->send_message() you need to do it, it has nothing to > do with client handling but has to do with the way spice-server > handles threading. If you don't do it you will create thread problems. [Kasireddy, Vivek] It appears sending these messages is resulting in gl_scanout and gl_draw items getting added to the pipe leading to the error message and client disconnection in the following function: RedPipeItemPtr dcc_gl_scanout_item_new(RedChannelClient *rcc, void *data, int num) { /* FIXME: on !unix peer, start streaming with a video codec */ if (!red_stream_is_plain_unix(rcc->get_stream()) || !rcc->test_remote_cap(SPICE_DISPLAY_CAP_GL_SCANOUT)) { red_channel_warning(rcc->get_channel(), "FIXME: client does not support GL scanout"); rcc->disconnect(); return RedPipeItemPtr(); } Thanks, Vivek > There's a "spice_threading_model.txt" document inside the "docs" > directory. Basically it's a message for the DisplayChannel thread to > do something. > > > > > > > > The internal function red_qxl_dmabuf_encode_async_complete is only > > > > used by the Spice server to indicate completion of the encoding process > > > > and send the completion cookie to applications." > > > > > > > > > > That's what red_qxl_gl_draw_async_complete is doing. > > [Kasireddy, Vivek] Yeah, this one, I can totally reuse. > > > > > > > > > > > > > > > > Cc: Gerd Hoffmann > > > > > > Cc: Marc-André Lureau > > > > > > Cc: Dongwon Kim > > > > > > Signed-off-by: Vivek Kasireddy > > > > > > --- > > > > > > server/display-channel.cpp | 9 + > > > > > > server/display-channel.h | 2 ++ > > > > > > server/red-qxl.cpp | 26 ++ > > > > > > server/red-qxl.h | 1 + > > > > > > server/spice-qxl.h | 2 ++ > > > > > > server/spice-server.syms | 1 + > > > > > > 6 files changed, 41 insertions(+) > > > > > > > > > > > > diff --git a/server/display-channel.cpp b/server/display-channel.cpp > > > > > > index 4bd0cf41..81df5420 100644 > > > > > > --- a/server/display-channel.cpp > > > > > > +++ b/server/display-channel.cpp > > > > > > @@ -2334,6 +2334,15 @@ void > > > > > display_channel_gl_draw_done(DisplayChannel *display) > > > > > > set_gl_draw_async_count(display, display->priv- > >gl_draw_async_count - > > > 1); > > > > > > } > > > > > > > > > > > > +void display_channel_encode_done(DisplayChannel *display, > > > > > > + RedDrawable *red_drawable) > > > > > > +{ > > > > > > +if (red_drawable->dmabuf_fd > 0) { > > > > > > +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); > > > > > > +red_
Re: [Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
Il giorno ven 13 gen 2023 alle ore 04:08 Kasireddy, Vivek ha scritto: > > Hi Frediano, > > > > > Il giorno gio 12 gen 2023 alle ore 07:03 Kasireddy, Vivek > > ha scritto: > > > > > > Hi Frediano, > > > > > > > > > > > Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy > > > > ha scritto: > > > > > > > > > > This async is triggered by the encoder indicating that the > > > > > encoding process is completed. > > > > > > > > > > > > > This description does not make much sense to me. > > > > You are adding a public function which, I suppose, should be called by > > > > Qemu but you are stating the encoder is calling it. > > > > Unless Qemu is the encoder it does not make sense. > > > [Kasireddy, Vivek] Sorry for the poor, misleading description. I > > > originally > > > had this patch split into two, one for each async function and forgot to > > > update the commit message when I merged them. I'll update the > > > commit message in v2 which would probably have the following desc: > > > "This patch mainly adds two async functions: a public one and an > > > internal one. The public function spice_qxl_dmabuf_encode_async is > > > used by applications that would share their fd with the Spice server. > > > > I don't see a reason for the new API, it's just doing a combination of > > spice_qxl_gl_scanout + spice_qxl_gl_draw_async. > [Kasireddy, Vivek] I had considered reusing these functions for my use-case > but decided against cluttering them. Anyway, do you think it is ok to add a > new parameter or a flag to these public APIs to prevent them from doing > qxl_state->send_message() because my use-case does not involve sending > any messages (neither RedWorkerMessageGlDraw nor > RedWorkerMessageGlScanout) to the client. > On the other end you are cluttering public API, Qemu and user usage. To use local connection you have to setup the VM in a different way than remote connection so you would have either to ask the administrator to reconfigure and restart the VM or having a possible suboptimal (or not working) connection. Doing inside spice-server won't require VM setup change (because spice-server can choose the way of handling it). About qxl_state->send_message() you need to do it, it has nothing to do with client handling but has to do with the way spice-server handles threading. If you don't do it you will create thread problems. There's a "spice_threading_model.txt" document inside the "docs" directory. Basically it's a message for the DisplayChannel thread to do something. > > > > > The internal function red_qxl_dmabuf_encode_async_complete is only > > > used by the Spice server to indicate completion of the encoding process > > > and send the completion cookie to applications." > > > > > > > That's what red_qxl_gl_draw_async_complete is doing. > [Kasireddy, Vivek] Yeah, this one, I can totally reuse. > > > > > > > > > > > > Cc: Gerd Hoffmann > > > > > Cc: Marc-André Lureau > > > > > Cc: Dongwon Kim > > > > > Signed-off-by: Vivek Kasireddy > > > > > --- > > > > > server/display-channel.cpp | 9 + > > > > > server/display-channel.h | 2 ++ > > > > > server/red-qxl.cpp | 26 ++ > > > > > server/red-qxl.h | 1 + > > > > > server/spice-qxl.h | 2 ++ > > > > > server/spice-server.syms | 1 + > > > > > 6 files changed, 41 insertions(+) > > > > > > > > > > diff --git a/server/display-channel.cpp b/server/display-channel.cpp > > > > > index 4bd0cf41..81df5420 100644 > > > > > --- a/server/display-channel.cpp > > > > > +++ b/server/display-channel.cpp > > > > > @@ -2334,6 +2334,15 @@ void > > > > display_channel_gl_draw_done(DisplayChannel *display) > > > > > set_gl_draw_async_count(display, > > > > > display->priv->gl_draw_async_count - > > 1); > > > > > } > > > > > > > > > > +void display_channel_encode_done(DisplayChannel *display, > > > > > + RedDrawable *red_drawable) > > > > > +{ > > > > > +if (red_drawable->dmabuf_fd > 0) { > > > > > +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); > > > > > +red_drawable->dmabuf_fd = 0; > > > > > +} > > > > > +} > > > > > + > > > > > int display_channel_get_video_stream_id(DisplayChannel *display, > > > > VideoStream *stream) > > > > > { > > > > > return static_cast(stream - > > > > > display->priv->streams_buf.data()); > > > > > diff --git a/server/display-channel.h b/server/display-channel.h > > > > > index c54df25c..0a1e520c 100644 > > > > > --- a/server/display-channel.h > > > > > +++ b/server/display-channel.h > > > > > @@ -127,6 +127,8 @@ void > > > > > display_channel_gl_scanout > > > > (DisplayCha > > > > > void display_channel_gl_draw > > > > > (DisplayChannel > > > > *display, > > > > > > > > > > SpiceMsgDisplayGlDraw *draw); > > > > > void display_ch
Re: [Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
Hi Frediano, > > Il giorno gio 12 gen 2023 alle ore 07:03 Kasireddy, Vivek > ha scritto: > > > > Hi Frediano, > > > > > > > > Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy > > > ha scritto: > > > > > > > > This async is triggered by the encoder indicating that the > > > > encoding process is completed. > > > > > > > > > > This description does not make much sense to me. > > > You are adding a public function which, I suppose, should be called by > > > Qemu but you are stating the encoder is calling it. > > > Unless Qemu is the encoder it does not make sense. > > [Kasireddy, Vivek] Sorry for the poor, misleading description. I originally > > had this patch split into two, one for each async function and forgot to > > update the commit message when I merged them. I'll update the > > commit message in v2 which would probably have the following desc: > > "This patch mainly adds two async functions: a public one and an > > internal one. The public function spice_qxl_dmabuf_encode_async is > > used by applications that would share their fd with the Spice server. > > I don't see a reason for the new API, it's just doing a combination of > spice_qxl_gl_scanout + spice_qxl_gl_draw_async. [Kasireddy, Vivek] I had considered reusing these functions for my use-case but decided against cluttering them. Anyway, do you think it is ok to add a new parameter or a flag to these public APIs to prevent them from doing qxl_state->send_message() because my use-case does not involve sending any messages (neither RedWorkerMessageGlDraw nor RedWorkerMessageGlScanout) to the client. > > > The internal function red_qxl_dmabuf_encode_async_complete is only > > used by the Spice server to indicate completion of the encoding process > > and send the completion cookie to applications." > > > > That's what red_qxl_gl_draw_async_complete is doing. [Kasireddy, Vivek] Yeah, this one, I can totally reuse. > > > > > > > > Cc: Gerd Hoffmann > > > > Cc: Marc-André Lureau > > > > Cc: Dongwon Kim > > > > Signed-off-by: Vivek Kasireddy > > > > --- > > > > server/display-channel.cpp | 9 + > > > > server/display-channel.h | 2 ++ > > > > server/red-qxl.cpp | 26 ++ > > > > server/red-qxl.h | 1 + > > > > server/spice-qxl.h | 2 ++ > > > > server/spice-server.syms | 1 + > > > > 6 files changed, 41 insertions(+) > > > > > > > > diff --git a/server/display-channel.cpp b/server/display-channel.cpp > > > > index 4bd0cf41..81df5420 100644 > > > > --- a/server/display-channel.cpp > > > > +++ b/server/display-channel.cpp > > > > @@ -2334,6 +2334,15 @@ void > > > display_channel_gl_draw_done(DisplayChannel *display) > > > > set_gl_draw_async_count(display, > > > > display->priv->gl_draw_async_count - > 1); > > > > } > > > > > > > > +void display_channel_encode_done(DisplayChannel *display, > > > > + RedDrawable *red_drawable) > > > > +{ > > > > +if (red_drawable->dmabuf_fd > 0) { > > > > +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); > > > > +red_drawable->dmabuf_fd = 0; > > > > +} > > > > +} > > > > + > > > > int display_channel_get_video_stream_id(DisplayChannel *display, > > > VideoStream *stream) > > > > { > > > > return static_cast(stream - > > > > display->priv->streams_buf.data()); > > > > diff --git a/server/display-channel.h b/server/display-channel.h > > > > index c54df25c..0a1e520c 100644 > > > > --- a/server/display-channel.h > > > > +++ b/server/display-channel.h > > > > @@ -127,6 +127,8 @@ void > > > > display_channel_gl_scanout > > > (DisplayCha > > > > void display_channel_gl_draw > > > > (DisplayChannel > > > *display, > > > > > > > > SpiceMsgDisplayGlDraw *draw); > > > > void display_channel_gl_draw_done > > > > (DisplayChannel > > > *display); > > > > +void display_channel_encode_done > > > > (DisplayChannel > > > *display, > > > > + > > > > RedDrawable *drawable); > > > > > > > > void display_channel_process_draw(DisplayChannel *display, > > > >red::shared_ptr > > > > &&red_drawable, > > > > diff --git a/server/red-qxl.cpp b/server/red-qxl.cpp > > > > index 48c293ae..42a4029b 100644 > > > > --- a/server/red-qxl.cpp > > > > +++ b/server/red-qxl.cpp > > > > @@ -493,6 +493,32 @@ void > red_qxl_gl_draw_async_complete(QXLInstance > > > *qxl) > > > > red_qxl_async_complete(qxl, cookie); > > > > } > > > > > > > > +SPICE_GNUC_VISIBLE > > > > +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, > > > > + int fd, uint64_t cookie) > > > > +{ > > > > +QXLState *qxl_state; > > > > + > > > > +spice_return_if_fail(qxl
Re: [Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
Il giorno gio 12 gen 2023 alle ore 07:03 Kasireddy, Vivek ha scritto: > > Hi Frediano, > > > > > Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy > > ha scritto: > > > > > > This async is triggered by the encoder indicating that the > > > encoding process is completed. > > > > > > > This description does not make much sense to me. > > You are adding a public function which, I suppose, should be called by > > Qemu but you are stating the encoder is calling it. > > Unless Qemu is the encoder it does not make sense. > [Kasireddy, Vivek] Sorry for the poor, misleading description. I originally > had this patch split into two, one for each async function and forgot to > update the commit message when I merged them. I'll update the > commit message in v2 which would probably have the following desc: > "This patch mainly adds two async functions: a public one and an > internal one. The public function spice_qxl_dmabuf_encode_async is > used by applications that would share their fd with the Spice server. I don't see a reason for the new API, it's just doing a combination of spice_qxl_gl_scanout + spice_qxl_gl_draw_async. > The internal function red_qxl_dmabuf_encode_async_complete is only > used by the Spice server to indicate completion of the encoding process > and send the completion cookie to applications." > That's what red_qxl_gl_draw_async_complete is doing. > > > > > Cc: Gerd Hoffmann > > > Cc: Marc-André Lureau > > > Cc: Dongwon Kim > > > Signed-off-by: Vivek Kasireddy > > > --- > > > server/display-channel.cpp | 9 + > > > server/display-channel.h | 2 ++ > > > server/red-qxl.cpp | 26 ++ > > > server/red-qxl.h | 1 + > > > server/spice-qxl.h | 2 ++ > > > server/spice-server.syms | 1 + > > > 6 files changed, 41 insertions(+) > > > > > > diff --git a/server/display-channel.cpp b/server/display-channel.cpp > > > index 4bd0cf41..81df5420 100644 > > > --- a/server/display-channel.cpp > > > +++ b/server/display-channel.cpp > > > @@ -2334,6 +2334,15 @@ void > > display_channel_gl_draw_done(DisplayChannel *display) > > > set_gl_draw_async_count(display, display->priv->gl_draw_async_count > > > - 1); > > > } > > > > > > +void display_channel_encode_done(DisplayChannel *display, > > > + RedDrawable *red_drawable) > > > +{ > > > +if (red_drawable->dmabuf_fd > 0) { > > > +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); > > > +red_drawable->dmabuf_fd = 0; > > > +} > > > +} > > > + > > > int display_channel_get_video_stream_id(DisplayChannel *display, > > VideoStream *stream) > > > { > > > return static_cast(stream - display->priv->streams_buf.data()); > > > diff --git a/server/display-channel.h b/server/display-channel.h > > > index c54df25c..0a1e520c 100644 > > > --- a/server/display-channel.h > > > +++ b/server/display-channel.h > > > @@ -127,6 +127,8 @@ void display_channel_gl_scanout > > (DisplayCha > > > void display_channel_gl_draw > > > (DisplayChannel > > *display, > > > > > > SpiceMsgDisplayGlDraw *draw); > > > void display_channel_gl_draw_done > > > (DisplayChannel > > *display); > > > +void display_channel_encode_done > > > (DisplayChannel > > *display, > > > + > > > RedDrawable *drawable); > > > > > > void display_channel_process_draw(DisplayChannel *display, > > >red::shared_ptr > > > &&red_drawable, > > > diff --git a/server/red-qxl.cpp b/server/red-qxl.cpp > > > index 48c293ae..42a4029b 100644 > > > --- a/server/red-qxl.cpp > > > +++ b/server/red-qxl.cpp > > > @@ -493,6 +493,32 @@ void red_qxl_gl_draw_async_complete(QXLInstance > > *qxl) > > > red_qxl_async_complete(qxl, cookie); > > > } > > > > > > +SPICE_GNUC_VISIBLE > > > +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, > > > + int fd, uint64_t cookie) > > > +{ > > > +QXLState *qxl_state; > > > + > > > +spice_return_if_fail(qxl != nullptr); > > > +qxl_state = qxl->st; > > > + > > > +qxl_state->scanout.drm_dma_buf_fd = fd; > > > +qxl_state->gl_draw_cookie = cookie; > > > > This behaviour is prone to leak resources. > [Kasireddy, Vivek] I guess I could do what spice_qxl_gl_scanout does: > that is, prevent the (Gstreamer) encoder from closing the fd and instead > do the following here: > pthread_mutex_lock(&qxl_state->scanout_mutex); > > if (qxl_state->scanout.drm_dma_buf_fd >= 0) { > close(qxl_state->scanout.drm_dma_buf_fd); > } > > Do you think this would help? > > Thanks, > Vivek > > > > > > +} > > > + > > > +void red_qxl_dmabuf_encode_async_complete(QXLInstance *qxl) > > > +{ > > > +
Re: [Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
Hi Frediano, > > Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy > ha scritto: > > > > This async is triggered by the encoder indicating that the > > encoding process is completed. > > > > This description does not make much sense to me. > You are adding a public function which, I suppose, should be called by > Qemu but you are stating the encoder is calling it. > Unless Qemu is the encoder it does not make sense. [Kasireddy, Vivek] Sorry for the poor, misleading description. I originally had this patch split into two, one for each async function and forgot to update the commit message when I merged them. I'll update the commit message in v2 which would probably have the following desc: "This patch mainly adds two async functions: a public one and an internal one. The public function spice_qxl_dmabuf_encode_async is used by applications that would share their fd with the Spice server. The internal function red_qxl_dmabuf_encode_async_complete is only used by the Spice server to indicate completion of the encoding process and send the completion cookie to applications." > > > Cc: Gerd Hoffmann > > Cc: Marc-André Lureau > > Cc: Dongwon Kim > > Signed-off-by: Vivek Kasireddy > > --- > > server/display-channel.cpp | 9 + > > server/display-channel.h | 2 ++ > > server/red-qxl.cpp | 26 ++ > > server/red-qxl.h | 1 + > > server/spice-qxl.h | 2 ++ > > server/spice-server.syms | 1 + > > 6 files changed, 41 insertions(+) > > > > diff --git a/server/display-channel.cpp b/server/display-channel.cpp > > index 4bd0cf41..81df5420 100644 > > --- a/server/display-channel.cpp > > +++ b/server/display-channel.cpp > > @@ -2334,6 +2334,15 @@ void > display_channel_gl_draw_done(DisplayChannel *display) > > set_gl_draw_async_count(display, display->priv->gl_draw_async_count - > > 1); > > } > > > > +void display_channel_encode_done(DisplayChannel *display, > > + RedDrawable *red_drawable) > > +{ > > +if (red_drawable->dmabuf_fd > 0) { > > +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); > > +red_drawable->dmabuf_fd = 0; > > +} > > +} > > + > > int display_channel_get_video_stream_id(DisplayChannel *display, > VideoStream *stream) > > { > > return static_cast(stream - display->priv->streams_buf.data()); > > diff --git a/server/display-channel.h b/server/display-channel.h > > index c54df25c..0a1e520c 100644 > > --- a/server/display-channel.h > > +++ b/server/display-channel.h > > @@ -127,6 +127,8 @@ void display_channel_gl_scanout > (DisplayCha > > void display_channel_gl_draw > > (DisplayChannel > *display, > > > > SpiceMsgDisplayGlDraw *draw); > > void display_channel_gl_draw_done > > (DisplayChannel > *display); > > +void display_channel_encode_done > > (DisplayChannel > *display, > > + > > RedDrawable *drawable); > > > > void display_channel_process_draw(DisplayChannel *display, > >red::shared_ptr > > &&red_drawable, > > diff --git a/server/red-qxl.cpp b/server/red-qxl.cpp > > index 48c293ae..42a4029b 100644 > > --- a/server/red-qxl.cpp > > +++ b/server/red-qxl.cpp > > @@ -493,6 +493,32 @@ void red_qxl_gl_draw_async_complete(QXLInstance > *qxl) > > red_qxl_async_complete(qxl, cookie); > > } > > > > +SPICE_GNUC_VISIBLE > > +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, > > + int fd, uint64_t cookie) > > +{ > > +QXLState *qxl_state; > > + > > +spice_return_if_fail(qxl != nullptr); > > +qxl_state = qxl->st; > > + > > +qxl_state->scanout.drm_dma_buf_fd = fd; > > +qxl_state->gl_draw_cookie = cookie; > > This behaviour is prone to leak resources. [Kasireddy, Vivek] I guess I could do what spice_qxl_gl_scanout does: that is, prevent the (Gstreamer) encoder from closing the fd and instead do the following here: pthread_mutex_lock(&qxl_state->scanout_mutex); if (qxl_state->scanout.drm_dma_buf_fd >= 0) { close(qxl_state->scanout.drm_dma_buf_fd); } Do you think this would help? Thanks, Vivek > > > +} > > + > > +void red_qxl_dmabuf_encode_async_complete(QXLInstance *qxl) > > +{ > > +QXLState *qxl_state = qxl->st; > > +uint64_t cookie = qxl->st->gl_draw_cookie; > > + > > +if (cookie == GL_DRAW_COOKIE_INVALID) { > > +return; > > +} > > +qxl_state->scanout.drm_dma_buf_fd = 0; > > +qxl->st->gl_draw_cookie = GL_DRAW_COOKIE_INVALID; > > +red_qxl_async_complete(qxl, cookie); > > +} > > + > > SPICE_GNUC_VISIBLE > > void spice_qxl_set_device_info(QXLInstance *instance, > > const char *device_address, > > di
Re: [Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy ha scritto: > > This async is triggered by the encoder indicating that the > encoding process is completed. > This description does not make much sense to me. You are adding a public function which, I suppose, should be called by Qemu but you are stating the encoder is calling it. Unless Qemu is the encoder it does not make sense. > Cc: Gerd Hoffmann > Cc: Marc-André Lureau > Cc: Dongwon Kim > Signed-off-by: Vivek Kasireddy > --- > server/display-channel.cpp | 9 + > server/display-channel.h | 2 ++ > server/red-qxl.cpp | 26 ++ > server/red-qxl.h | 1 + > server/spice-qxl.h | 2 ++ > server/spice-server.syms | 1 + > 6 files changed, 41 insertions(+) > > diff --git a/server/display-channel.cpp b/server/display-channel.cpp > index 4bd0cf41..81df5420 100644 > --- a/server/display-channel.cpp > +++ b/server/display-channel.cpp > @@ -2334,6 +2334,15 @@ void display_channel_gl_draw_done(DisplayChannel > *display) > set_gl_draw_async_count(display, display->priv->gl_draw_async_count - 1); > } > > +void display_channel_encode_done(DisplayChannel *display, > + RedDrawable *red_drawable) > +{ > +if (red_drawable->dmabuf_fd > 0) { > +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); > +red_drawable->dmabuf_fd = 0; > +} > +} > + > int display_channel_get_video_stream_id(DisplayChannel *display, VideoStream > *stream) > { > return static_cast(stream - display->priv->streams_buf.data()); > diff --git a/server/display-channel.h b/server/display-channel.h > index c54df25c..0a1e520c 100644 > --- a/server/display-channel.h > +++ b/server/display-channel.h > @@ -127,6 +127,8 @@ void display_channel_gl_scanout >(DisplayCha > void display_channel_gl_draw > (DisplayChannel *display, > > SpiceMsgDisplayGlDraw *draw); > void display_channel_gl_draw_done > (DisplayChannel *display); > +void display_channel_encode_done > (DisplayChannel *display, > + > RedDrawable *drawable); > > void display_channel_process_draw(DisplayChannel *display, >red::shared_ptr > &&red_drawable, > diff --git a/server/red-qxl.cpp b/server/red-qxl.cpp > index 48c293ae..42a4029b 100644 > --- a/server/red-qxl.cpp > +++ b/server/red-qxl.cpp > @@ -493,6 +493,32 @@ void red_qxl_gl_draw_async_complete(QXLInstance *qxl) > red_qxl_async_complete(qxl, cookie); > } > > +SPICE_GNUC_VISIBLE > +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, > + int fd, uint64_t cookie) > +{ > +QXLState *qxl_state; > + > +spice_return_if_fail(qxl != nullptr); > +qxl_state = qxl->st; > + > +qxl_state->scanout.drm_dma_buf_fd = fd; > +qxl_state->gl_draw_cookie = cookie; This behaviour is prone to leak resources. > +} > + > +void red_qxl_dmabuf_encode_async_complete(QXLInstance *qxl) > +{ > +QXLState *qxl_state = qxl->st; > +uint64_t cookie = qxl->st->gl_draw_cookie; > + > +if (cookie == GL_DRAW_COOKIE_INVALID) { > +return; > +} > +qxl_state->scanout.drm_dma_buf_fd = 0; > +qxl->st->gl_draw_cookie = GL_DRAW_COOKIE_INVALID; > +red_qxl_async_complete(qxl, cookie); > +} > + > SPICE_GNUC_VISIBLE > void spice_qxl_set_device_info(QXLInstance *instance, > const char *device_address, > diff --git a/server/red-qxl.h b/server/red-qxl.h > index 2084acb1..e8e7c373 100644 > --- a/server/red-qxl.h > +++ b/server/red-qxl.h > @@ -40,6 +40,7 @@ bool red_qxl_get_allow_client_mouse(QXLInstance *qxl, int > *x_res, int *y_res, in > SpiceMsgDisplayGlScanoutUnix *red_qxl_get_gl_scanout(QXLInstance *qxl); > void red_qxl_put_gl_scanout(QXLInstance *qxl, SpiceMsgDisplayGlScanoutUnix > *scanout); > void red_qxl_gl_draw_async_complete(QXLInstance *qxl); > +void red_qxl_dmabuf_encode_async_complete(QXLInstance *qxl); > int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int minor); > SpiceServer* red_qxl_get_server(QXLState *qxl); > uint32_t red_qxl_marshall_device_display_info(const QXLInstance *qxl, > SpiceMarshaller *m); > diff --git a/server/spice-qxl.h b/server/spice-qxl.h > index bf17476b..ca9816ec 100644 > --- a/server/spice-qxl.h > +++ b/server/spice-qxl.h > @@ -92,6 +92,8 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl, > uint32_t x, uint32_t y, > uint32_t w, uint32_t h, > uint64_t cookie); > +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, > + int fd, uint64_t cookie); > > /* since spice 0.14.2
[Spice-devel] [RFC v1 2/4] display-channel: Add the asyncs associated with dmabuf encode
This async is triggered by the encoder indicating that the encoding process is completed. Cc: Gerd Hoffmann Cc: Marc-André Lureau Cc: Dongwon Kim Signed-off-by: Vivek Kasireddy --- server/display-channel.cpp | 9 + server/display-channel.h | 2 ++ server/red-qxl.cpp | 26 ++ server/red-qxl.h | 1 + server/spice-qxl.h | 2 ++ server/spice-server.syms | 1 + 6 files changed, 41 insertions(+) diff --git a/server/display-channel.cpp b/server/display-channel.cpp index 4bd0cf41..81df5420 100644 --- a/server/display-channel.cpp +++ b/server/display-channel.cpp @@ -2334,6 +2334,15 @@ void display_channel_gl_draw_done(DisplayChannel *display) set_gl_draw_async_count(display, display->priv->gl_draw_async_count - 1); } +void display_channel_encode_done(DisplayChannel *display, + RedDrawable *red_drawable) +{ +if (red_drawable->dmabuf_fd > 0) { +red_qxl_dmabuf_encode_async_complete(display->priv->qxl); +red_drawable->dmabuf_fd = 0; +} +} + int display_channel_get_video_stream_id(DisplayChannel *display, VideoStream *stream) { return static_cast(stream - display->priv->streams_buf.data()); diff --git a/server/display-channel.h b/server/display-channel.h index c54df25c..0a1e520c 100644 --- a/server/display-channel.h +++ b/server/display-channel.h @@ -127,6 +127,8 @@ void display_channel_gl_scanout (DisplayCha void display_channel_gl_draw (DisplayChannel *display, SpiceMsgDisplayGlDraw *draw); void display_channel_gl_draw_done (DisplayChannel *display); +void display_channel_encode_done (DisplayChannel *display, + RedDrawable *drawable); void display_channel_process_draw(DisplayChannel *display, red::shared_ptr &&red_drawable, diff --git a/server/red-qxl.cpp b/server/red-qxl.cpp index 48c293ae..42a4029b 100644 --- a/server/red-qxl.cpp +++ b/server/red-qxl.cpp @@ -493,6 +493,32 @@ void red_qxl_gl_draw_async_complete(QXLInstance *qxl) red_qxl_async_complete(qxl, cookie); } +SPICE_GNUC_VISIBLE +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, + int fd, uint64_t cookie) +{ +QXLState *qxl_state; + +spice_return_if_fail(qxl != nullptr); +qxl_state = qxl->st; + +qxl_state->scanout.drm_dma_buf_fd = fd; +qxl_state->gl_draw_cookie = cookie; +} + +void red_qxl_dmabuf_encode_async_complete(QXLInstance *qxl) +{ +QXLState *qxl_state = qxl->st; +uint64_t cookie = qxl->st->gl_draw_cookie; + +if (cookie == GL_DRAW_COOKIE_INVALID) { +return; +} +qxl_state->scanout.drm_dma_buf_fd = 0; +qxl->st->gl_draw_cookie = GL_DRAW_COOKIE_INVALID; +red_qxl_async_complete(qxl, cookie); +} + SPICE_GNUC_VISIBLE void spice_qxl_set_device_info(QXLInstance *instance, const char *device_address, diff --git a/server/red-qxl.h b/server/red-qxl.h index 2084acb1..e8e7c373 100644 --- a/server/red-qxl.h +++ b/server/red-qxl.h @@ -40,6 +40,7 @@ bool red_qxl_get_allow_client_mouse(QXLInstance *qxl, int *x_res, int *y_res, in SpiceMsgDisplayGlScanoutUnix *red_qxl_get_gl_scanout(QXLInstance *qxl); void red_qxl_put_gl_scanout(QXLInstance *qxl, SpiceMsgDisplayGlScanoutUnix *scanout); void red_qxl_gl_draw_async_complete(QXLInstance *qxl); +void red_qxl_dmabuf_encode_async_complete(QXLInstance *qxl); int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int minor); SpiceServer* red_qxl_get_server(QXLState *qxl); uint32_t red_qxl_marshall_device_display_info(const QXLInstance *qxl, SpiceMarshaller *m); diff --git a/server/spice-qxl.h b/server/spice-qxl.h index bf17476b..ca9816ec 100644 --- a/server/spice-qxl.h +++ b/server/spice-qxl.h @@ -92,6 +92,8 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl, uint32_t x, uint32_t y, uint32_t w, uint32_t h, uint64_t cookie); +void spice_qxl_dmabuf_encode_async(QXLInstance *qxl, + int fd, uint64_t cookie); /* since spice 0.14.2 */ diff --git a/server/spice-server.syms b/server/spice-server.syms index 8da46c20..9748cc24 100644 --- a/server/spice-server.syms +++ b/server/spice-server.syms @@ -182,4 +182,5 @@ SPICE_SERVER_0.14.3 { global: spice_server_get_video_codecs; spice_server_free_video_codecs; +spice_qxl_dmabuf_encode_async; } SPICE_SERVER_0.14.2; -- 2.37.2