"Verbeiren, David" <david.verbei...@intel.com> writes:
> Thanks for your comments, Anthony.
>
> Anthony Liguori wrote:
>> David Verbeiren <david.verbei...@intel.com> writes:
>> > This patch provides the VNC server side support. Corresponding VNC
>> > client side support is required. To this end, we are also contributing a
>> > patch
>> > to the libvncserver project (more specifically its libvncclient subproject)
>> > which can be used to test this experimental feature.
>> FWIW, the most common VNC client used with QEMU is gtk-vnc. That's the
>> widget used by vinagre, GNOME Boxes, and the various virt-tools
>> (virt-manager, virt-viewer).
>>
>> Implementing support in gtk-vnc isn't a prerequisite for merging into
>> QEMU, but thought you might be interested in the above.
>
> That's a good point. We'll look at enabling gtk-vnc as well.
>
>> You appear to be using an unallocated encoding number though. You can
>> get your own encoding number by contacting Tristan Richardson from
>> RealVNC or I'd be happy to use one of my encoding numbers...
>
> Thanks for pointing that out. I'll try to get an encoding number
> allocated for this.
>
>> > + /* Open local X11 display and VA display */
>> > + h264->x11_dpy = XOpenDisplay(":0.0");
>> > + if (!h264->x11_dpy) {
>> > + fprintf(stderr, "Could not open display :0.0 for VA encoding\n");
>> > + return -1;
>> > + }
>> > + int va_major_ver, va_minor_ver;
>> > + h264->va_dpy = vaGetDisplay(h264->x11_dpy);
>> > + va_status = vaInitialize(h264->va_dpy, &va_major_ver, &va_minor_ver);
>> > + CHECK_VASTATUS(va_status, "vaInitialize");
>>
>> So you need an X server in order to use libva? That's a little
>> disappointing as it
>> would be nice to use this acceleration on a server without X installed.
>> Is there any way to avoid the X interaction?
>
> I believe libva versions currently under development allow headless
> operation and removed that dependency on X (e.g. for Wayland support).
> So this requirement will go away at some point.
> We could conditionally compile for different versions of libva but that
> would make the code quite messy...
>
>> > + static struct {
>> > + unsigned char *data;
>> > + int length;
>> > + int is_intra;
>> > + unsigned long count;
>> > + } frame = { .data = NULL, .length = -1, .is_intra = -1, .count =
>> > -1 };
>>
>> Why is this static? We can have multiple clients connected at once so I'm
>> pretty sure
>> this would break really badly.
>
> The way support for multiple clients is implemented is by sending the
> same H264 data to all clients. This avoids having to run encoding
> (the heavy part) for each.
> When a new client connects, we force sending an I-frame so it will get a
> self-contained frame to start from.
> Hence this data is in fact unique per qemu process by design.
> We'll look at making this cleaner though.
>
>> > + if (!vs->vd->h264.va_dpy || (vs->vd->h264.config ==
>> > VA_INVALID_ID)) {
>> > + if (h264_encoder_init(&vs->vd->h264) != 0) {
>> > + fprintf(stderr, "Failed to initialize VA H264 encoder\n");
>> > + return 0;
>> > + }
>> Since we really can't guarantee that libva will initialize okay, we need
>> to more gracefully handle falling back to another encoding type.
>
> I'll see if we can move the critical parts of init to an earlier stage
> where we could still fall back.
>
>> Is this pseudo-encoding documented somewhere publicly?
>> It should be if it already isn't.
>
> No, it currently isn't. Any recommendation on where this could go?
The folks at TigerVNC maintain a community spec that includes extensions
that are not part of the original specification:
http://sourceforge.net/apps/mediawiki/tigervnc/index.php?title=Welcome_to_TigerVNC#RFB_Protocol
Regards,
Anthony Liguori
>
>> It looks mostly okay otherwise.
>
> I'll follow your recommendation on the other points you raised and
> submit an updated patch.
>
> Thanks,
> -David
> Intel Corporation NV/SA
> Kings Square, Veldkant 31
> 2550 Kontich
> RPM (Bruxelles) 0415.497.718.
> Citibank, Brussels, account 570/1031255/09
>
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). Any review or distribution by others
> is strictly prohibited. If you are not the intended recipient, please contact
> the sender and delete all copies.
