Il giorno mar 24 gen 2023 alle ore 06:41 Kasireddy, Vivek
<vivek.kasire...@intel.com> ha scritto:
>
> + Frediano
>
> Hi Gerd,
>
> >
> > Hi,
> >
> > > Here is the flow of things from the Qemu side:
> > > - Call gl_scanout (to update the fd) and gl_draw_async just like
> > > in the local display case.
> >
> > Ok.
> >
> > > - Additionally, create an update with the cmd set to QXL_CMD_DRAW
> > > to trigger the creation of a new drawable (associated with the fd)
> > > by the Spice server.
> > > - Wait (or block) until the Encoder is done encoding the content.
> > > - Unblock the pipeline once the async completion cookie is received.
> >
> > Care to explain? For qemu it should make a difference what spice-server
> > does with the dma-bufs passed (local display / encode video + send to
> > remote).
> [Kasireddy, Vivek] I agree that Qemu shouldn't care what the spice-server does
> with the dmabuf fds but somehow a drawable has to be created in the remote
> client
> case. This is needed as most of the core functions in the server (associated
> with
> display-channel, video-stream, encoder, etc) operate on drawables. Therefore,
> I
> figured since Qemu already tells the server to create a drawable in the
> non-gl case
> (by creating an update that includes a QXL_CMD_DRAW cmd), the same thing
> can be done in the gl + remote client case as well.
>
This is a hack. It's combining an invalid command in order to cause
some side effects on spice server but it also needs a change in spice
server to detect and handle this hack. QXL_CMD_DRAW is mainly bound to
2D commands and should come with valid bitmap data.
> Alternatively, we could make the server create a drawable as a response to
> gl_scanout,
> when it detects a remote client. IIUC, I think this can be done but seems
> rather messy
> given that currently, the server only creates a drawable (inside
> red_process_display)
> in the case of QXL_CMD_DRAW sent by Qemu/applications:
> switch (ext_cmd.cmd.type) {
> case QXL_CMD_DRAW: {
> auto red_drawable = red_drawable_new(worker->qxl,
> &worker->mem_slots,
> ext_cmd.group_id,
> ext_cmd.cmd.data,
> ext_cmd.flags); // returns
> with 1 ref
>
> if (red_drawable) {
> display_channel_process_draw(worker->display_channel,
> std::move(red_drawable),
>
> worker->process_display_generation);
> }
>
Yes, it sounds a bit long but surely better than hacking Qemu, spice
server and defining a new hacky ABI that will be hard to maintain and
understand.
> The other option I can think of is to just not deal with drawables at all and
> somehow
> directly share the dmabuf fd with the Encoder. This solution also seems very
> messy
> and invasive to me as we'd not be able to leverage the existing APIs (in
> display-channel,
> video-stream, etc) that create and manage streams efficiently.
>
Yes, that currently seems pretty long as a change but possibly the
most clean in future, it's up to some refactory. The Encoder does not
either need technically a RedDrawable, Drawable but frame data encoded
in a format it can manage (either raw memory data or dmabuf at the
moment).
> >
> > > #ifdef HAVE_SPICE_GL
> > > + } else if (spice_dmabuf_encode) {
> > > + if (g_strcmp0(preferred_codec, "gstreamer:h264")) {
> > > + error_report("dmabuf-encode=on currently only works and
> > > tested"
> > > + "with gstreamer:h264");
> > > + exit(1);
> > > + }
> >
> > IMHO we should not hard-code todays spice-server capabilities like this.
> > For starters this isn't true for spice-server versions which don't (yet)
> > have your patches. Also the capability might depend on hardware
> > support. IMHO we need some feature negotiation between qemu and spice
> > here.
> [Kasireddy, Vivek] Ok, I can get rid of this chunk in v3. However, given the
> numerous
> features supported by the Spice server, I suspect implementing feature
> negotiation
> might get really challenging. Is there any other way around this that you can
> think of?
>
> Thanks,
> Vivek
>
> >
> > take care,
> > Gerd
>
Frediano
