ui/vnc: VA API based H.264 encoding for VNC framebuffer updates

This patch implements H.264 encoding of the VNC framebuffer updates
using hardware acceleration through the VA API.

This is experimental support to let the community explore the possibilities
offered by the potential bandwidth and latency reductions that H.264 encoding
allows. This may be particularly useful for use cases such as online gaming,
hosted desktops, hosted set top boxes...
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.
See instructions below for how to build a test VNC client.

In case multiple regions are updated, only the first framebuffer
update message of the batch carries the H.264 frame data.
Subsequent update framebuffer messages will contain only the
coordinates and size of the other updated regions.

This is backwards compatible with standard VNC clients thanks to
the encoding scheme negotiation included in VNC. If the client doesn't
support H.264 encoding, the server will fall back to one of the usual
VNC encodings.

Instructions/Requirements:
* Currently only works with libva 1.0: use branch "v1.0-branch" for libva and
intel-driver. Those can be built as follows:
   cd libva
   git checkout v1.0-branch
   ./autogen.sh
   make
   sudo make install
   cd ..
   git clone git://anongit.freedesktop.org/vaapi/intel-driver
   cd intel-driver
   git checkout v1.0-branch
   ./autogen.sh
   make
   sudo make install
* A graphical environment must be running as the v1.0-branch of VA API does not
support headless operation.
* When using Intel integrated graphics, hardware encoding support requires a
2nd generation (or later) i3, i5 or i7 processor ("Sandy Bridge" or later), or
similar, with enabled Intel(R) HD graphics.
See http://intellinuxgraphics.org/h264.html for details.

Instructions for building and using test client:
* Get LibVNCServer project
   git clone git://libvncserver.git.sourceforge.net/gitroot/libvncserver/libvncserver
   git checkout 55bdab02574e3ac
* Download and apply (git apply <patch_file>) following two patches in sequence:
   http://sourceforge.net/mailarchive/message.php?msg_id=30323804
   http://sourceforge.net/mailarchive/message.php?msg_id=30327573
* Run the client, for example:
   ./client_examples/gtkvncviewer vm_host:5901

Signed-off-by: David Verbeiren <david.verbeiren@intel.com>
---
 configure         |   39 ++++
 ui/Makefile.objs  |    1 +
 ui/vnc-enc-h264.c |  592 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ui/vnc-enc-h264.h |   61 ++++++
 ui/vnc-jobs.c     |    6 +
 ui/vnc.c          |   22 ++
 ui/vnc.h          |   17 ++
 7 files changed, 738 insertions(+)
 create mode 100644 ui/vnc-enc-h264.c
 create mode 100644 ui/vnc-enc-h264.h

Hi,

David Verbeiren <david.verbeiren@intel.com> writes:

> This patch implements H.264 encoding of the VNC framebuffer updates
> using hardware acceleration through the VA API.
>
> This is experimental support to let the community explore the possibilities
> offered by the potential bandwidth and latency reductions that H.264 encoding
> allows. This may be particularly useful for use cases such as online gaming,
> hosted desktops, hosted set top boxes...

That's pretty awesome!

Do you have any benchmark results?  Are there any limitations in the
number of QEMU instances that can use the hardware acceleration
simultaneously?

> 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.
> See instructions below for how to build a test VNC client.

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).

https://live.gnome.org/gtk-vnc

Implementing support in gtk-vnc isn't a prerequisite for merging into
QEMU, but thought you might be interested in the above.

> In case multiple regions are updated, only the first framebuffer
> update message of the batch carries the H.264 frame data.
> Subsequent update framebuffer messages will contain only the
> coordinates and size of the other updated regions.
>
> This is backwards compatible with standard VNC clients thanks to
> the encoding scheme negotiation included in VNC. If the client doesn't
> support H.264 encoding, the server will fall back to one of the usual
> VNC encodings.

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...

> Instructions/Requirements:
> * Currently only works with libva 1.0: use branch "v1.0-branch" for libva and
> intel-driver. Those can be built as follows:
>    cd libva
>    git checkout v1.0-branch
>    ./autogen.sh
>    make
>    sudo make install
>    cd ..
>    git clone git://anongit.freedesktop.org/vaapi/intel-driver
>    cd intel-driver
>    git checkout v1.0-branch
>    ./autogen.sh
>    make
>    sudo make install
> * A graphical environment must be running as the v1.0-branch of VA API does not
> support headless operation.
> * When using Intel integrated graphics, hardware encoding support requires a
> 2nd generation (or later) i3, i5 or i7 processor ("Sandy Bridge" or later), or
> similar, with enabled Intel(R) HD graphics.
> See http://intellinuxgraphics.org/h264.html for details.
>
> Instructions for building and using test client:
> * Get LibVNCServer project
>    git clone git://libvncserver.git.sourceforge.net/gitroot/libvncserver/libvncserver
>    git checkout 55bdab02574e3ac
> * Download and apply (git apply <patch_file>) following two patches in sequence:
>    http://sourceforge.net/mailarchive/message.php?msg_id=30323804
>    http://sourceforge.net/mailarchive/message.php?msg_id=30327573
> * Run the client, for example:
>    ./client_examples/gtkvncviewer vm_host:5901
>
> Signed-off-by: David Verbeiren <david.verbeiren@intel.com>
> ---
>  configure         |   39 ++++
>  ui/Makefile.objs  |    1 +
>  ui/vnc-enc-h264.c |  592 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ui/vnc-enc-h264.h |   61 ++++++
>  ui/vnc-jobs.c     |    6 +
>  ui/vnc.c          |   22 ++
>  ui/vnc.h          |   17 ++
>  7 files changed, 738 insertions(+)
>  create mode 100644 ui/vnc-enc-h264.c
>  create mode 100644 ui/vnc-enc-h264.h
>
> diff --git a/configure b/configure
> index fe18ed2..566f5aa 100755
> --- a/configure
> +++ b/configure
> @@ -212,6 +212,7 @@ pie=""
>  zero_malloc=""
>  trace_backend="nop"
>  trace_file="trace"
> +libva="yes"

You should default to "" which only enables the feature if libva is available.

>  spice=""
>  rbd=""
>  smartcard=""
> @@ -763,6 +764,10 @@ for opt do
>    ;;
>    --enable-spice) spice="yes"
>    ;;
> +  --disable-libva) libva="no"
> +  ;;
> +  --enable-libva) libva="yes"
> +  ;;
>    --disable-libiscsi) libiscsi="no"
>    ;;
>    --enable-libiscsi) libiscsi="yes"
> @@ -1123,6 +1128,8 @@ echo "  --with-trace-file=NAME   Full PATH,NAME of file to store traces"
>  echo "                           Default:trace-<pid>"
>  echo "  --disable-spice          disable spice"
>  echo "  --enable-spice           enable spice"
> +echo "  --disable-libva          disable libva"
> +echo "  --enable-libva           enable libva"
>  echo "  --enable-rbd             enable building the rados block device (rbd)"
>  echo "  --disable-libiscsi       disable iscsi support"
>  echo "  --enable-libiscsi        enable iscsi support"
> @@ -2810,6 +2817,33 @@ EOF
>    fi
>  fi
>  
> +##########################################
> +# libva probe
> +if test "$libva" != "no" ; then
> +  cat > $TMPC << EOF
> +#include <X11/Xlib.h>
> +#include <va/va.h>
> +#include <va/va_x11.h>
> +int main(void) { int major_ver, minor_ver;
> + Display *win_display = (Display *)XOpenDisplay(":0.0");
> + VADisplay va_dpy = vaGetDisplay(win_display);
> + vaInitialize(va_dpy, &major_ver, &minor_ver); return 0;
> +}
> +EOF
> +libva_cflags="$($pkg_config --cflags libva 2>/dev/null) $($pkg_config --cflags libva-x11 2>/dev/null) $($pkg_config --cflags x11 2>/dev/null)"
> +libva_libs="$($pkg_config --libs libva 2>/dev/null) $($pkg_config --libs libva-x11 2>/dev/null) $($pkg_config --libs x11 2>/dev/null)"
> +  if $pkg_config --atleast-version=0.26 libva >/dev/null 2>&1 && \
> +     compile_prog "$libva_cflags" "$libva_libs" ; then
> +    libva="yes"
> +    libs_softmmu="$libs_softmmu $libva_libs"
> +  else
> +    if test "$libva" = "yes" ; then
> +      feature_not_found "libva"
> +    fi
> +    libva="no"
> +  fi
> +fi
> +
>  # check for libcacard for smartcard support
>  if test "$smartcard" != "no" ; then
>      smartcard="yes"
> @@ -3311,6 +3345,7 @@ echo "xfsctl support    $xfs"
>  echo "nss used          $smartcard_nss"
>  echo "usb net redir     $usb_redir"
>  echo "OpenGL support    $opengl"
> +echo "libva support     $libva"
>  echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
>  echo "seccomp support   $seccomp"
> @@ -3592,6 +3627,10 @@ if test "$spice" = "yes" ; then
>    echo "CONFIG_SPICE=y" >> $config_host_mak
>  fi
>  
> +if test "$libva" = "yes" ; then
> +  echo "CONFIG_VNC_LIBVA=y" >> $config_host_mak
> +fi
> +
>  if test "$smartcard" = "yes" ; then
>    echo "CONFIG_SMARTCARD=y" >> $config_host_mak
>  fi
> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index 6768bb7..ff37ec9 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -4,6 +4,7 @@ vnc-obj-y += vnc-enc-tight.o vnc-palette.o
>  vnc-obj-y += vnc-enc-zrle.o
>  vnc-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
>  vnc-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
> +vnc-obj-$(CONFIG_VNC_LIBVA) += vnc-enc-h264.o
>  vnc-obj-y += vnc-jobs.o
>  
>  common-obj-y += keymaps.o console.o cursor.o input.o qemu-pixman.o
> diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
> new file mode 100644
> index 0000000..89253a9
> --- /dev/null
> +++ b/ui/vnc-enc-h264.c
> @@ -0,0 +1,592 @@
> +/*
> + * QEMU VNC display driver: VAAPI based H.264 encoding
> + *
> + * Copyright (c) 2012 Intel Corporation. All Rights Reserved.

Do you intend to have this 'All Rights Reserved' bit?  That seems to
contract with the license grant below...

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "vnc.h"
> +
> +#define CHECK_VASTATUS(va_status, func) do {                                   \
> +    if (va_status != VA_STATUS_SUCCESS) {                                      \
> +        fprintf(stderr, "%s:%s:%d failed (0x%x),exit\n", __func__, func,       \
> +                __LINE__, va_status);                                          \
> +        exit(1);                                                               \
> +    } else {                                                                   \
> +        /* fprintf(stderr, "%s:%s:%d success\n", __func__, func, __LINE__); */ \
> +    }                                                                          \
> +} while (0);
> +
> +
> +/* Forward declarations */
> +static int h264_encoder_init(VncDisplayH264 *h264);
> +static int h264_encoder_reinit(VncDisplayH264 *h264, int w, int h, int pitch,
> +                               int depth);
> +static void h264_encoder_close(VncDisplayH264 *h264);
> +
> +static int h264_create_context(VncDisplayH264 *h264, int width, int height,
> +                               int pitch, int depth);
> +static void h264_release_context(VncDisplayH264 *h264);
> +
> +static void h264_prepare_input(VncDisplayH264 *h264,
> +                               const unsigned char *const in_buf,
> +                               const bool is_intra, const bool is_last);
> +static void h264_encode_frame(VncDisplayH264 *h264,
> +                              const unsigned char *const in_buf, const int last,
> +                              const bool is_intra, int *out_length,
> +                              unsigned char **out_buf);
> +static void rgba_to_surface(const VncDisplayH264 *h264,
> +                            const unsigned char *const inbuf,
> +                            VASurfaceID surface_id);
> +static int get_buffer_length(const unsigned char * const buffer,
> +                             int buffer_length);
> +
> +
> +static int h264_encoder_init(VncDisplayH264 *h264)
> +{
> +    if (h264->x11_dpy) {
> +        fprintf(stderr, "%s called, but encoder is already initialized\n",
> +                __func__);
> +        return -1;
> +    }
> +
> +    VAStatus va_status;
> +
> +    /* 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?

> +    /* Check for Slice entrypoint */
> +    VAEntrypoint entrypoints[5];
> +    int num_entrypoints, entrypoint;

We prefer to keep type declarations at the top of functions (same as
kernel style).

> +
> +    vaQueryConfigEntrypoints(h264->va_dpy, VAProfileH264Baseline, entrypoints,
> +                             &num_entrypoints);
> +
> +    for (entrypoint = 0; entrypoint < num_entrypoints; entrypoint++) {
> +        if (entrypoints[entrypoint] == VAEntrypointEncSlice) {
> +            break;
> +        }
> +    }
> +
> +    if (entrypoint == num_entrypoints) {
> +        VNC_DEBUG("Could not find Encoder Slice entrypoint\n");
> +        h264_encoder_close(h264);
> +        return -1;
> +    }
> +
> +    /* Set encode pipeline configuration */
> +    VAConfigAttrib attrib[2];
> +
> +    attrib[0].type = VAConfigAttribRTFormat;
> +    attrib[1].type = VAConfigAttribRateControl;
> +    vaGetConfigAttributes(h264->va_dpy, VAProfileH264Baseline,
> +                          VAEntrypointEncSlice, &attrib[0], 2);
> +
> +    if ((attrib[0].value & VA_RT_FORMAT_YUV420) == 0) {
> +        VNC_DEBUG("YUV420 format is not supported by VA encoder\n");
> +        h264_encoder_close(h264);
> +        return -1;
> +    }
> +    if ((attrib[1].value & VA_RC_VBR) == 0) {
> +        VNC_DEBUG("VBR mode is not supported by VA encoder\n");
> +        h264_encoder_close(h264);
> +        return -1;
> +    }
> +
> +    attrib[0].value = VA_RT_FORMAT_YUV420;
> +    attrib[1].value = VA_RC_VBR;
> +    va_status = vaCreateConfig(h264->va_dpy, VAProfileH264Baseline,
> +                               VAEntrypointEncSlice, &attrib[0], 2,
> +                               &h264->config);
> +    CHECK_VASTATUS(va_status, "vaCreateConfig");
> +
> +    /* Invalidate context */
> +    h264->context = VA_INVALID_ID;
> +    h264->seq_param = VA_INVALID_ID;
> +    h264->pic_param = VA_INVALID_ID;
> +    h264->slice_param = VA_INVALID_ID;
> +    h264->coded_buf = VA_INVALID_ID;
> +
> +    int i;
> +    for (i = 0; i < SID_NUMBER; ++i) {
> +        h264->surface_ids[i] = VA_INVALID_ID;
> +    }
> +
> +    /* Misc */
> +    h264->frame_count = 0;
> +
> +    return 0;
> +}
> +
> +static int h264_encoder_reinit(VncDisplayH264 *h264, int w, int h, int pitch,
> +                               int depth)
> +{
> +    h264_release_context(h264);
> +
> +    return h264_create_context(h264, w, h, pitch, depth);
> +}
> +
> +static void h264_encoder_close(VncDisplayH264 *h264)
> +{
> +    if (h264->config != VA_INVALID_ID) {
> +        vaDestroyConfig(h264->va_dpy, h264->config);
> +        h264->config = VA_INVALID_ID;
> +    }
> +    if (h264->va_dpy) {
> +        vaTerminate(h264->va_dpy);
> +        h264->va_dpy = NULL;
> +    }
> +    if (h264->x11_dpy) {
> +        XCloseDisplay(h264->x11_dpy);
> +        h264->x11_dpy = NULL;
> +    }
> +}
> +
> +int vnc_h264_send_framebuffer_update(VncState *vs, int ch_x,
> +                                     int ch_y, int ch_w, int ch_h)
> +{
> +    VNC_DEBUG("%s: x=%d; y=%d; w=%d; h=%d\n", __func__, ch_x, ch_y,
> +              ch_w, ch_h);
> +
> +    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.

> +
> +    int frame_w = ds_get_width(vs->ds);
> +    int frame_h = ds_get_height(vs->ds);
> +
> +    /* first call for this frame: perform encoding */
> +    if ((frame.data == NULL) || (frame.count != vs->vd->h264.frame_count)) {
> +
> +        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.

> +        }
> +
> +        static int old_w = -1;
> +        static int old_h = -1;

More statics that need to go away.

> +        if ((frame_w != old_w) || (frame_h != old_h)) {
> +            int pitch = ds_get_linesize(vs->ds);
> +            int depth = ds_get_bytes_per_pixel(vs->ds);
> +
> +            VNC_DEBUG("%s: Resolution change: (%dx%d) => (%dx%d) "
> +                      "(depth: %d, pitch: %d)\n", __func__,
> +                      old_w, old_h, frame_w, frame_h, depth, pitch);
> +
> +            if (h264_encoder_reinit(&vs->vd->h264, frame_w, frame_h,
> +                                    pitch, depth) != 0) {
> +                VNC_DEBUG("Error re-initializing the VA encoder\n");
> +                return 0;
> +            }
> +
> +            /* send full screen update on framebuffer resize/init */
> +            vs->vd->h264.force_intra = true;
> +
> +            old_w = frame_w;
> +            old_h = frame_h;
> +        }
> +
> +        /* Encode the frame and get the encoded frame and its size */
> +        frame.is_intra = vs->vd->h264.force_intra ||
> +                                 ((vs->vd->h264.frame_count % 30) == 0);
> +        h264_encode_frame(&vs->vd->h264, vnc_server_fb_ptr(vs->vd, 0, 0), 0,
> +                          frame.is_intra, &frame.length, &frame.data);
> +
> +        frame.count = vs->vd->h264.frame_count;
> +        vs->vd->h264.force_intra = false;
> +    }
> +
> +    /* Send framebuffer update header for this rectangle */
> +    vnc_framebuffer_update(vs, ch_x, ch_y, ch_w, ch_h, VNC_ENCODING_H264);
> +
> +    /* Send frame contents only once. Multiple updates for the same frame
> +     * are sent when there are multiple changed rectangles in the same
> +     * framebuffer update run (see vnc_update_client() in vnc.c). */
> +    int bytes_to_send =
> +                  (vs->h264_last_sent_frame != frame.count) ? frame.length : 0;
> +
> +    vnc_write_s32(vs, bytes_to_send);
> +    vnc_write_s32(vs, frame.is_intra ? 2 : 0);
> +    vnc_write_s32(vs, frame_w);
> +    vnc_write_s32(vs, frame_h);
> +    vnc_write(vs, frame.data, bytes_to_send);
> +
> +    vs->h264_last_sent_frame = frame.count;
> +
> +    return 1;
> +}

Is this pseudo-encoding documented somewhere publicly?  It should be if
it already isn't.

> +
> +
> +static int h264_create_context(VncDisplayH264 *h264, int width, int height,
> +                               int pitch, int depth)
> +{
> +    VNC_DEBUG("%s: width=%d; height=%d; pitch=%d; depth=%d\n", __func__,
> +              width, height, pitch, depth);
> +
> +    if (h264->context != VA_INVALID_ID) {
> +        VNC_DEBUG("%s: context already exists, doing nothing\n", __func__);
> +        return 0;
> +    }
> +
> +    VAStatus va_status;
> +
> +    /* Set picture related parameters */
> +    h264->pic_width = width;
> +    h264->pic_height = height;
> +    h264->pic_depth = depth;
> +    h264->pic_pitch = pitch;
> +
> +    /* Create context */
> +    va_status = vaCreateContext(h264->va_dpy, h264->config, width, height,
> +                                VA_PROGRESSIVE, 0, 0, &h264->context);
> +    CHECK_VASTATUS(va_status, "vaCreateContext");
> +
> +    /* Sequence parameters */
> +    VAEncSequenceParameterBufferH264 seq_h264 = {0};
> +    seq_h264.level_idc = 30;
> +    seq_h264.picture_width_in_mbs = (width + 15) / 16;
> +    seq_h264.picture_height_in_mbs = (height + 15) / 16;
> +    seq_h264.bits_per_second = 384 * 1000;
> +    seq_h264.initial_qp = 26;
> +    seq_h264.min_qp = 3;
> +
> +    va_status = vaCreateBuffer(h264->va_dpy, h264->context,
> +                               VAEncSequenceParameterBufferType,
> +                               sizeof(seq_h264), 1, &seq_h264,
> +                               &h264->seq_param);
> +    CHECK_VASTATUS(va_status, "vaCreateBuffer(seq_param)");;
> +
> +    /* Surfaces */
> +    va_status = vaCreateSurfaces(h264->va_dpy, width, height,
> +                                 VA_RT_FORMAT_YUV420, SID_NUMBER,
> +                                 &h264->surface_ids[0]);
> +    CHECK_VASTATUS(va_status, "vaCreateSurfaces");
> +    VNC_DEBUG("%s: Created surfaces:", __func__);
> +    int i;
> +    for (i = 0; i < SID_NUMBER; ++i) {
> +        VNC_DEBUG(" [#%d -> 0x%x]", i, h264->surface_ids[i]);
> +    }
> +    VNC_DEBUG("\n");
> +
> +    /* Coded buffer */
> +    unsigned int coded_buf_size = width * height * 1.5;
> +    va_status = vaCreateBuffer(h264->va_dpy, h264->context,
> +                               VAEncCodedBufferType, coded_buf_size, 1, NULL,
> +                               &h264->coded_buf);
> +    CHECK_VASTATUS(va_status, "vaCreateBuffer(coded_buf)");
> +    VNC_DEBUG("%s: created coded_buf: id=0x%x, size=%d\n", __func__,
> +              h264->coded_buf, coded_buf_size);
> +
> +    return 0;
> +}
> +
> +static void h264_release_context(VncDisplayH264 *h264)
> +{
> +    if (h264->context == VA_INVALID_ID) {
> +        return;
> +    }
> +
> +    VAStatus va_status;
> +
> +    /* Sync all surfaces */
> +    VNC_DEBUG("%s: Syncing surfaces: ", __func__);
> +    int i;
> +    for (i = 0; i < SID_NUMBER; ++i) {
> +        if (h264->surface_ids[i] != VA_INVALID_ID) {
> +            VNC_DEBUG("[#%d", i);
> +
> +            va_status = vaSyncSurface(h264->va_dpy, h264->surface_ids[i]);
> +            CHECK_VASTATUS(va_status, "vaSyncSurface");
> +
> +            VASurfaceStatus surface_status = (VASurfaceStatus)0;
> +            va_status = vaQuerySurfaceStatus(h264->va_dpy, h264->surface_ids[i],
> +                                             &surface_status);
> +            CHECK_VASTATUS(va_status, "vaQuerySurfaceStatus");
> +
> +            VNC_DEBUG(" -> 0x%x] ", surface_status);
> +        }
> +    }
> +    VNC_DEBUG("done!\n");
> +
> +    /* Release parameter buffers */
> +    VNC_DEBUG("%s: seq_param = 0x%x\n", __func__, h264->seq_param);
> +    if (h264->seq_param != VA_INVALID_ID) {
> +        va_status = vaDestroyBuffer(h264->va_dpy, h264->seq_param);
> +        CHECK_VASTATUS(va_status, "vaDestroyBuffer(seq_param)");
> +        h264->seq_param = VA_INVALID_ID;
> +    }
> +
> +    VNC_DEBUG("%s: pic_param = 0x%x\n", __func__, h264->pic_param);
> +    if (h264->pic_param != VA_INVALID_ID) {
> +        va_status = vaDestroyBuffer(h264->va_dpy, h264->pic_param);
> +        CHECK_VASTATUS(va_status, "vaDestroyBuffer(pic_param)");
> +        h264->pic_param = VA_INVALID_ID;
> +    }
> +
> +    VNC_DEBUG("%s: slice_param = 0x%x\n", __func__, h264->slice_param);
> +    if (h264->slice_param != VA_INVALID_ID) {
> +        va_status = vaDestroyBuffer(h264->va_dpy, h264->slice_param);
> +        CHECK_VASTATUS(va_status, "vaDestroyBuffer(slice_param)");
> +        h264->slice_param = VA_INVALID_ID;
> +    }
> +
> +    VNC_DEBUG("%s: coded_buf = 0x%x\n", __func__, h264->coded_buf);
> +    if (h264->coded_buf != VA_INVALID_ID) {
> +        va_status = vaDestroyBuffer(h264->va_dpy, h264->coded_buf);
> +        CHECK_VASTATUS(va_status, "vaDestroyBuffer(coded_buf)");
> +        h264->coded_buf = VA_INVALID_ID;
> +    }
> +
> +    /* Release surfaces */
> +    if (h264->surface_ids[0] != VA_INVALID_ID) {
> +        va_status = vaDestroySurfaces(h264->va_dpy, &h264->surface_ids[0],
> +                                      SID_NUMBER);
> +        CHECK_VASTATUS(va_status, "vaDestroySurfaces");
> +    }
> +    for (i = 0; i < SID_NUMBER; ++i) {
> +        h264->surface_ids[i] = VA_INVALID_ID;
> +    }
> +
> +    /* Release context */
> +    va_status = vaDestroyContext(h264->va_dpy, h264->context);
> +    CHECK_VASTATUS(va_status, "vaDestroyContext");
> +    h264->context = VA_INVALID_ID;
> +}
> +
> +
> +static void h264_encode_frame(VncDisplayH264 *h264,
> +                              const unsigned char *const in_buf, const int last,
> +                              const bool is_intra, int *out_length,
> +                              unsigned char **out_buf)
> +{
> +    VAStatus va_status;
> +
> +    va_status = vaBeginPicture(h264->va_dpy, h264->context,
> +                               h264->surface_ids[SID_INPUT_PICTURE]);
> +    CHECK_VASTATUS(va_status, "vaBeginPicture");
> +
> +    /* Set parameters and put picture on the input picture surface */
> +    h264_prepare_input(h264, in_buf, is_intra, last);
> +
> +    /* Sync input surface */
> +    va_status = vaEndPicture(h264->va_dpy, h264->context);
> +    CHECK_VASTATUS(va_status, "vaEndPicture");
> +
> +    va_status = vaSyncSurface(h264->va_dpy,
> +                              h264->surface_ids[SID_INPUT_PICTURE]);
> +    CHECK_VASTATUS(va_status, "vaSyncSurface");
> +
> +    VASurfaceStatus surface_status;
> +    surface_status = (VASurfaceStatus)0;
> +    va_status = vaQuerySurfaceStatus(h264->va_dpy,
> +                                     h264->surface_ids[SID_INPUT_PICTURE],
> +                                     &surface_status);
> +    CHECK_VASTATUS(va_status, "vaQuerySurfaceStatus");
> +
> +    /* Get H.264 encoded data */
> +    if (h264->coded_buf != VA_INVALID_ID) {
> +        va_status = vaUnmapBuffer(h264->va_dpy, h264->coded_buf);
> +        CHECK_VASTATUS(va_status, "vaUnmapBuffer(coded_buf)");
> +    }
> +
> +    VACodedBufferSegment *coded_buf_segment = NULL;
> +
> +    va_status = vaMapBuffer(h264->va_dpy, h264->coded_buf,
> +                            (void **)(&coded_buf_segment));
> +    CHECK_VASTATUS(va_status, "vaMapBuffer");
> +
> +    *out_buf = (unsigned char *)coded_buf_segment->buf;
> +
> +    if (coded_buf_segment->next) {
> +        VNC_DEBUG("%s: Unsupported: multiple segments detected. "
> +                  "Skipping next segments\n", __func__);
> +    }
> +
> +    int coded_buf_size = h264->pic_width * h264->pic_height * 1.5;
> +    *out_length = get_buffer_length(*out_buf, coded_buf_size);
> +}
> +
> +
> +static void h264_prepare_input(VncDisplayH264 *h264,
> +                               const unsigned char *const in_buf,
> +                               const bool is_intra, const bool is_last)
> +{
> +    VNC_DEBUG("%s: in_buf=%p; is_intra=%d; is_last=%d\n", __func__, in_buf,
> +              is_intra, is_last);
> +
> +    VAStatus va_status;
> +
> +    /* Set sequence parameters */
> +    va_status = vaRenderPicture(h264->va_dpy, h264->context,
> +                                &h264->seq_param, 1);
> +    CHECK_VASTATUS(va_status, "vaRenderPicture(seq_param)");
> +
> +    /* Set picture parameters */
> +    static VAEncPictureParameterBufferH264 pic_h264;

More statics.

It looks mostly okay otherwise.

Regards,

Anthony Liguori

> +
> +    pic_h264.reference_picture = h264->surface_ids[SID_REFERENCE_PICTURE];
> +    pic_h264.reconstructed_picture = h264->surface_ids[SID_RECON_PICTURE];
> +    pic_h264.coded_buf = h264->coded_buf;
> +    pic_h264.picture_width = h264->pic_width;
> +    pic_h264.picture_height = h264->pic_height;
> +    pic_h264.last_picture = is_last;
> +
> +    if (h264->pic_param != VA_INVALID_ID) {
> +        va_status = vaDestroyBuffer(h264->va_dpy, h264->pic_param);
> +        CHECK_VASTATUS(va_status, "vaDestroyBuffer(pic_parameter)");
> +    }
> +
> +    va_status = vaCreateBuffer(h264->va_dpy, h264->context,
> +                               VAEncPictureParameterBufferType,
> +                               sizeof(pic_h264), 1, &pic_h264,
> +                               &h264->pic_param);
> +    CHECK_VASTATUS(va_status, "vaCreateBuffer(pic_parameter)");
> +
> +    va_status = vaRenderPicture(h264->va_dpy, h264->context,
> +                                &h264->pic_param, 1);
> +    CHECK_VASTATUS(va_status, "vaRenderPicture(pic_parameter)");
> +
> +    /* Allocate and zero out coded buffer segment.
> +     * See also get_buffer_length(). */
> +    VACodedBufferSegment *coded_buf_segment = NULL;
> +    va_status = vaMapBuffer(h264->va_dpy, h264->coded_buf,
> +                            (void **)(&coded_buf_segment));
> +    CHECK_VASTATUS(va_status, "vaMapBuffer(coded_buf_segment)");
> +
> +    unsigned char *coded_mem = (unsigned char *)coded_buf_segment->buf;
> +    memset(coded_mem, 0, coded_buf_segment->size);
> +
> +    va_status = vaUnmapBuffer(h264->va_dpy, h264->coded_buf);
> +    CHECK_VASTATUS(va_status, "vaUnmapBuffer(coded_buf)");
> +
> +    /* Set slice parameters */
> +    static VAEncSliceParameterBuffer slice_h264;
> +
> +    slice_h264.start_row_number = 0;
> +    slice_h264.slice_height = h264->pic_height / 16;    /* Measured in MB */
> +    slice_h264.slice_flags.bits.is_intra = is_intra;
> +    slice_h264.slice_flags.bits.disable_deblocking_filter_idc = 0;
> +
> +    if (h264->slice_param != VA_INVALID_ID) {
> +        va_status = vaDestroyBuffer(h264->va_dpy, h264->slice_param);
> +        CHECK_VASTATUS(va_status, "vaDestroyBuffer(slice_parameter)");
> +    }
> +
> +    va_status = vaCreateBuffer(h264->va_dpy, h264->context,
> +                               VAEncSliceParameterBufferType,
> +                               sizeof(slice_h264), 1, &slice_h264,
> +                               &h264->slice_param);
> +    CHECK_VASTATUS(va_status, "vaCreateBuffer(slice_parameter)");
> +
> +    va_status = vaRenderPicture(h264->va_dpy, h264->context,
> +                                &h264->slice_param, 1);
> +    CHECK_VASTATUS(va_status, "vaRenderPicture(slice_parameter)");
> +
> +    /* Copy RGBA input buffer to VA surface */
> +    rgba_to_surface(h264, in_buf, h264->surface_ids[SID_INPUT_PICTURE]);
> +
> +    /* Prepare for next picture */
> +    VABufferID tempID = h264->surface_ids[SID_RECON_PICTURE];
> +    h264->surface_ids[SID_RECON_PICTURE] =
> +                                      h264->surface_ids[SID_REFERENCE_PICTURE];
> +    h264->surface_ids[SID_REFERENCE_PICTURE] = tempID;
> +}
> +
> +static void rgba_to_surface(const VncDisplayH264 *h264,
> +                            const unsigned char *const inbuf,
> +                            VASurfaceID surface_id)
> +{
> +    VNC_DEBUG("%s: inbuf=%p; width=%d; height=%d; frame=%ld\n", __func__,
> +              inbuf, h264->pic_width, h264->pic_height, h264->frame_count);
> +
> +    VAStatus va_status;
> +
> +    VAImage image;
> +    void *pbuffer = NULL;
> +    const unsigned char *psrc = inbuf;
> +    const unsigned char *s;
> +    unsigned char *pdst = NULL;
> +    unsigned char *dst_y, *dst_uv;
> +    unsigned char *dst_uv_line = NULL;
> +
> +    va_status = vaDeriveImage(h264->va_dpy, surface_id, &image);
> +    CHECK_VASTATUS(va_status, "vaDeriveImage");
> +
> +    va_status = vaMapBuffer(h264->va_dpy, image.buf, &pbuffer);
> +    CHECK_VASTATUS(va_status, "vaMapBuffer(image.buf)");
> +
> +    pdst = (unsigned char *)pbuffer;
> +    dst_uv_line = pdst + image.offsets[1];
> +
> +    /* RGBA => NV12 */
> +    int i, j;
> +    for (i = 0; i < h264->pic_height; ++i) {
> +        dst_y = (pdst + image.offsets[0]) + i*image.pitches[0];
> +        dst_uv = dst_uv_line;
> +        s = psrc;
> +        for (j = 0; j < h264->pic_width; j++) {
> +            *dst_y++ = ((66*s[0] + 129*s[1] + 25*s[2] + 128) >> 8) + 16;
> +            /* NV12 means subsampling by 2 in x and y */
> +            if ((0 == i%2) && (0 == j%2)) {
> +                *dst_uv++ = ((112*s[0] - 94*s[1] - 18*s[2] + 128) >> 8) + 128;
> +                *dst_uv++ = ((-38*s[0] - 74*s[1] + 112*s[2] + 128) >> 8) + 128;
> +            }
> +            s += h264->pic_depth;
> +        }
> +        psrc += h264->pic_pitch;
> +        if (0 == i%2) {
> +            dst_uv_line += image.pitches[1];
> +        }
> +    }
> +
> +    va_status = vaUnmapBuffer(h264->va_dpy, image.buf);
> +    CHECK_VASTATUS(va_status, "vaUnmapBuffer(image.buf)");
> +
> +    va_status = vaDestroyImage(h264->va_dpy, image.image_id);
> +    CHECK_VASTATUS(va_status, "vaDestroyImage");
> +}
> +
> +static int get_buffer_length(const unsigned char * const buffer,
> +                             int buffer_length)
> +{
> +    int i;
> +    for (i = buffer_length - 1; i >= 0; i--) {
> +        if (buffer[i]) {
> +            break;
> +        }
> +    }
> +
> +    return i + 1;
> +}
> diff --git a/ui/vnc-enc-h264.h b/ui/vnc-enc-h264.h
> new file mode 100644
> index 0000000..d6fc1f5
> --- /dev/null
> +++ b/ui/vnc-enc-h264.h
> @@ -0,0 +1,61 @@
> +/*
> + * QEMU VNC display driver: VAAPI based H.264 encoding
> + *
> + * Copyright (c) 2012 Intel Corporation. All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifndef __QEMU_VNC_ENC_H264_H__
> +#define __QEMU_VNC_ENC_H264_H__
> +
> +#include <X11/Xlib.h>
> +#include <va/va_x11.h>
> +
> +#define SID_NUMBER                              3
> +#define SID_INPUT_PICTURE                       0
> +#define SID_REFERENCE_PICTURE                   1
> +#define SID_RECON_PICTURE                       2
> +
> +typedef struct VncDisplayH264 {
> +    /* Encoding context related */
> +    Display *x11_dpy;
> +    VADisplay va_dpy;
> +    VAConfigID config;
> +    VAContextID context;
> +    VASurfaceID surface_ids[SID_NUMBER];
> +    VABufferID seq_param;       /* Sequence parameters */
> +    VABufferID pic_param;       /* Picture parameters */
> +    VABufferID slice_param;     /* Slice parameters */
> +    VABufferID coded_buf;       /* Output buffer, encoded data */
> +
> +    /* Picture related */
> +    int pic_width;
> +    int pic_height;
> +    int pic_depth;
> +    int pic_pitch;
> +
> +    /* Misc */
> +    unsigned long frame_count;
> +    bool force_intra;
> +} VncDisplayH264;
> +
> +#endif /* __QEMU_VNC_ENC_H264_H__ */
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 0bfc0c5..ad02e37 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -194,6 +194,9 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
>      local->hextile = orig->hextile;
>      local->zrle = orig->zrle;
>      local->output =  queue->buffer;
> +#ifdef CONFIG_VNC_LIBVA
> +    local->h264_last_sent_frame = orig->h264_last_sent_frame;
> +#endif
>      local->csock = -1; /* Don't do any network work on this thread */
>  
>      buffer_reset(&local->output);
> @@ -206,6 +209,9 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local)
>      orig->hextile = local->hextile;
>      orig->zrle = local->zrle;
>      orig->lossy_rect = local->lossy_rect;
> +#ifdef CONFIG_VNC_LIBVA
> +    orig->h264_last_sent_frame = local->h264_last_sent_frame;
> +#endif
>  
>      queue->buffer = local->output;
>  }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 8912b78..a4c1906 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -707,6 +707,11 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
>          case VNC_ENCODING_ZYWRLE:
>              n = vnc_zywrle_send_framebuffer_update(vs, x, y, w, h);
>              break;
> +#ifdef CONFIG_VNC_LIBVA
> +        case VNC_ENCODING_H264:
> +            n = vnc_h264_send_framebuffer_update(vs, x, y, w, h);
> +            break;
> +#endif
>          default:
>              vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
>              n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
> @@ -1821,6 +1826,12 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>              vs->features |= VNC_FEATURE_ZYWRLE_MASK;
>              vs->vnc_encoding = enc;
>              break;
> +#ifdef CONFIG_VNC_LIBVA
> +        case VNC_ENCODING_H264:
> +            vs->features |= VNC_FEATURE_H264_MASK;
> +            vs->vnc_encoding = enc;
> +            break;
> +#endif
>          case VNC_ENCODING_DESKTOPRESIZE:
>              vs->features |= VNC_FEATURE_RESIZE_MASK;
>              break;
> @@ -2616,6 +2627,12 @@ static void vnc_refresh(void *opaque)
>      has_dirty = vnc_refresh_server_surface(vd);
>      vnc_unlock_display(vd);
>  
> +#ifdef CONFIG_VNC_LIBVA
> +    if (has_dirty) {
> +        ++vd->h264.frame_count;
> +    }
> +#endif
> +
>      QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
>          rects += vnc_update_client(vs, has_dirty);
>          /* vs might be free()ed here */
> @@ -2705,6 +2722,11 @@ static void vnc_connect(VncDisplay *vd, int csock, int skipauth)
>  
>      QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
>  
> +#ifdef CONFIG_VNC_LIBVA
> +    vs->h264_last_sent_frame = vd->h264.frame_count - 1;
> +    vd->h264.force_intra = true;
> +#endif
> +
>      vga_hw_update();
>  
>      vnc_write(vs, "RFB 003.008\n", 12);
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 8b40f09..3bfee13 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -99,6 +99,9 @@ typedef struct VncDisplay VncDisplay;
>  #ifdef CONFIG_VNC_SASL
>  #include "vnc-auth-sasl.h"
>  #endif
> +#ifdef CONFIG_VNC_LIBVA
> +#include "vnc-enc-h264.h"
> +#endif
>  
>  struct VncRectStat
>  {
> @@ -167,6 +170,9 @@ struct VncDisplay
>  #ifdef CONFIG_VNC_SASL
>      VncDisplaySASL sasl;
>  #endif
> +#ifdef CONFIG_VNC_LIBVA
> +    VncDisplayH264 h264;
> +#endif
>  };
>  
>  typedef struct VncTight {
> @@ -303,6 +309,10 @@ struct VncState
>      VncZrle zrle;
>      VncZywrle zywrle;
>  
> +#ifdef CONFIG_VNC_LIBVA
> +    unsigned long h264_last_sent_frame;
> +#endif
> +
>      Notifier mouse_mode_notifier;
>  
>      QTAILQ_ENTRY(VncState) next;
> @@ -358,6 +368,7 @@ enum {
>  #define VNC_ENCODING_TRLE                 0x0000000f
>  #define VNC_ENCODING_ZRLE                 0x00000010
>  #define VNC_ENCODING_ZYWRLE               0x00000011
> +#define VNC_ENCODING_H264                 0x48323634
>  #define VNC_ENCODING_COMPRESSLEVEL0       0xFFFFFF00 /* -256 */
>  #define VNC_ENCODING_QUALITYLEVEL0        0xFFFFFFE0 /* -32  */
>  #define VNC_ENCODING_XCURSOR              0xFFFFFF10 /* -240 */
> @@ -407,6 +418,7 @@ enum {
>  #define VNC_FEATURE_TIGHT_PNG                8
>  #define VNC_FEATURE_ZRLE                     9
>  #define VNC_FEATURE_ZYWRLE                  10
> +#define VNC_FEATURE_H264                    11
>  
>  #define VNC_FEATURE_RESIZE_MASK              (1 << VNC_FEATURE_RESIZE)
>  #define VNC_FEATURE_HEXTILE_MASK             (1 << VNC_FEATURE_HEXTILE)
> @@ -419,6 +431,7 @@ enum {
>  #define VNC_FEATURE_TIGHT_PNG_MASK           (1 << VNC_FEATURE_TIGHT_PNG)
>  #define VNC_FEATURE_ZRLE_MASK                (1 << VNC_FEATURE_ZRLE)
>  #define VNC_FEATURE_ZYWRLE_MASK              (1 << VNC_FEATURE_ZYWRLE)
> +#define VNC_FEATURE_H264_MASK                (1 << VNC_FEATURE_H264)
>  
>  
>  /* Client -> Server message IDs */
> @@ -560,4 +573,8 @@ int vnc_zrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
>  int vnc_zywrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
>  void vnc_zrle_clear(VncState *vs);
>  
> +#ifdef CONFIG_VNC_LIBVA
> +int vnc_h264_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> +#endif
> +
>  #endif /* __QEMU_VNC_H */
> -- 
> 1.7.9.5
>
> 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.

On 09.01.2013, at 21:25, Anthony Liguori wrote:

> Hi,
> 
> David Verbeiren <david.verbeiren@intel.com> writes:

[...]

> 
>> +
>> +    int frame_w = ds_get_width(vs->ds);
>> +    int frame_h = ds_get_height(vs->ds);
>> +
>> +    /* first call for this frame: perform encoding */
>> +    if ((frame.data == NULL) || (frame.count != vs->vd->h264.frame_count)) {
>> +
>> +        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.

In fact, doesn't libavcodec / gstreamer support vaapi? If we could just send our updates to a library and have that determine whether vaapi is available or maybe fall back to software H.264 encoding, people without vaapi compatible hardware (or software) could exploit this feature too.

Though I'm not saying it's a prerequisite to merge this patch. As a proof of concept, it's good enough to rely on vaapi directly. I just think not doing so would open this to a broader range of users. And it would get rid of a whole class of problems (hardware limitations on concurrent users, X dependency, etc)


Alex

On 01/09/13 23:18, Alexander Graf wrote:
> 
> On 09.01.2013, at 21:25, Anthony Liguori wrote:
> 
>> 
>> Since we really can't guarantee that libva will initialize okay, we
>> need to more gracefully handle falling back to another encoding
>> type.
> 
> In fact, doesn't libavcodec / gstreamer support vaapi? If we could
> just send our updates to a library and have that determine whether
> vaapi is available or maybe fall back to software H.264 encoding,
> people without vaapi compatible hardware (or software) could exploit
> this feature too.

Using libavcodec directly is a non-starter as distros don't ship that
due to the multimedia codec patent mess.

gstreamer should be workable, and thanks to the plugin system you can
even use libavcodec indirectly if it happens to be installed (from 3rd
party repo).

cheers,
  Gerd

On Thu, 10 Jan 2013, Gerd Hoffmann wrote:
> On 01/09/13 23:18, Alexander Graf wrote:
> > 
> > On 09.01.2013, at 21:25, Anthony Liguori wrote:
> > 
> >> 
> >> Since we really can't guarantee that libva will initialize okay, we
> >> need to more gracefully handle falling back to another encoding
> >> type.
> > 
> > In fact, doesn't libavcodec / gstreamer support vaapi? If we could
> > just send our updates to a library and have that determine whether
> > vaapi is available or maybe fall back to software H.264 encoding,
> > people without vaapi compatible hardware (or software) could exploit
> > this feature too.
> 
> Using libavcodec directly is a non-starter as distros don't ship that
> due to the multimedia codec patent mess.

libavcodec is certainly available on my ubuntu and debian installs.

On 01/10/13 11:37, Stefano Stabellini wrote:
> On Thu, 10 Jan 2013, Gerd Hoffmann wrote:
>> On 01/09/13 23:18, Alexander Graf wrote:
>> Using libavcodec directly is a non-starter as distros don't ship that
>> due to the multimedia codec patent mess.
> 
> libavcodec is certainly available on my ubuntu and debian installs.

Full version with H.264 support?  Or stripped down, with patented codecs
removed?

cheers,
  Gerd

On 10.01.2013, at 12:10, Gerd Hoffmann wrote:

> On 01/10/13 11:37, Stefano Stabellini wrote:
>> On Thu, 10 Jan 2013, Gerd Hoffmann wrote:
>>> On 01/09/13 23:18, Alexander Graf wrote:
>>> Using libavcodec directly is a non-starter as distros don't ship that
>>> due to the multimedia codec patent mess.
>> 
>> libavcodec is certainly available on my ubuntu and debian installs.
> 
> Full version with H.264 support?  Or stripped down, with patented codecs
> removed?

IIRC Ubuntu has full support. Also openSUSE has Packman where you could get full libavcodec builds.

But overall, gstreamer is probably the better interface to look at here, as it actually provides a plugin architecture that people can plug their almost-legal codecs into.

The main reason I brought this up was because I'd like to know whether there's a particular benefit in using vaapi directly vs using gstreamer to encode this :).


Alex

On 01/10/13 12:19, Alexander Graf wrote:
> 
> On 10.01.2013, at 12:10, Gerd Hoffmann wrote:
> 
>> On 01/10/13 11:37, Stefano Stabellini wrote:
>>> On Thu, 10 Jan 2013, Gerd Hoffmann wrote:
>>>> On 01/09/13 23:18, Alexander Graf wrote: Using libavcodec
>>>> directly is a non-starter as distros don't ship that due to the
>>>> multimedia codec patent mess.
>>> 
>>> libavcodec is certainly available on my ubuntu and debian
>>> installs.
>> 
>> Full version with H.264 support?  Or stripped down, with patented
>> codecs removed?
> 
> IIRC Ubuntu has full support. Also openSUSE has Packman where you
> could get full libavcodec builds.

Fedora has rpmfusion.  But packages in the distro core can't have direct
dependencies to 3rd party repo packages like
packman/rpmfusion/non-free/whatever-it-is-called-for-your-distro.

> But overall, gstreamer is probably the better interface to look at
> here, as it actually provides a plugin architecture that people can
> plug their almost-legal codecs into.

Exactly.  Taking the indirection via gstreamer plugins allows to build
qemu core distro package with H.264 support enabled.  Actually using
that requires the gstreamer plugins (for libavcodec and/or vaapi) being
installed of course.

cheers,
  Gerd

On 10/01/2013, 12:20 PM, Alexander Graf wrote:
> On 10.01.2013, at 12:10, Gerd Hoffmann wrote:
>> On 01/10/13 11:37, Stefano Stabellini wrote:
>>> On Thu, 10 Jan 2013, Gerd Hoffmann wrote:
>>>> On 01/09/13 23:18, Alexander Graf wrote:
>>>> Using libavcodec directly is a non-starter as distros don't ship that
>>>> due to the multimedia codec patent mess.
>>> 
>>> libavcodec is certainly available on my ubuntu and debian installs.
>> 
>> Full version with H.264 support?  Or stripped down, with patented codecs
>> removed?
>
> IIRC Ubuntu has full support. Also openSUSE has Packman where you could get
> full libavcodec builds.
>
> But overall, gstreamer is probably the better interface to look at here,
> as it actually provides a plugin architecture that people can plug their
> almost-legal codecs into.
>
> The main reason I brought this up was because I'd like to know whether
> there's a particular benefit in using vaapi directly vs using gstreamer to
> encode this :).

Thank you all for your interest in this.

First of all, this patch was meant to make it possible to experiment with
the approach without heavy lifting. As such, there are certainly other
alternatives that can be considered for the longer term.

Even though I can see big advantages in plugging into GStreamer for
streaming out the framebuffer, encoded as H.264 or other, I'm not sure
it would provide similar functionality to the experimental implementation
I provided, without significant additional efforts.

Currently, the patch implements sending the encoded data within
the "VNC channel", which means the data shows up naturally in the
VNC client.
If we instead use a separate mechanism, that GStreamer already supports,
to stream the framebuffer updates (e.g. RTP) to the client, an existing
VNC client would have to be enhanced with a full network stream input
support (+ extra bitstream parsing).
Alternatively, if we take an existing video player with network stream
support, we'd have to add to it full VNC protocol and user input support.
In both cases, this seems like a lot more effort to reach a functional
VNC client than what we've come up with.

Or maybe we could plug into GStreamer at both ends of its pipeline,
providing a GstSource at the input, feeding the pipeline with the
VM framebuffer updates, and a GstSink at the output to grab back
the encoded bitstream before sending it within the standard
"VNC channel".
We'd still have to add full bitstream parsing in the client,
but that probably makes the solution more standard anyway.

From this point of view, interfacing to libavcodec would seem to
potentially bring some of the advantages (no need to maintain own encoder
code that would need to evolve with VAAPI) without any impact on the
VNC side.

However, I'm not sure any of those libraries currently supports
VA API encoding (only decoding, I think). I know there has been some effort
towards that but I don't think it has been finalized yet. I'll check what
the latest status is there.

Regards,
-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.

Thanks for your comments, Anthony.

 Anthony Liguori wrote:
> David Verbeiren <david.verbeiren@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?

> 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.

"Verbeiren, David" <david.verbeiren@intel.com> writes:

> Thanks for your comments, Anthony.
>
>  Anthony Liguori wrote:
>> David Verbeiren <david.verbeiren@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.

Hello Qemu people,

2013/1/9 David Verbeiren <david.verbeiren@intel.com>:
> This patch implements H.264 encoding of the VNC framebuffer updates
> using hardware acceleration through the VA API.

Sounds Great !

Sorry if my question has a obvious response but I can't find it:
David, on which version of Qemu can we apply your attached patch ?

Thanks !

Hi Jérôme,

The patch applied cleanly on qemu master branch when I sent it. I see the addition of ui/vnc_ws probably broke it.
The specific commit I tested it against was 8e4a424b305e29...:
http://git.qemu.org/?p=qemu.git;a=snapshot;h=8e4a424b305e29dc0e454f52df3b35577f342975;sf=tgz

I'll also take care of that when I send a next revision addressing the comments I got from the list (hopefully some time next week).

Regards,
-David

-----Original Message-----
From: Jérôme Jutteau [mailto:jerome.jutteau@outscale.com] 

Sent: Thursday, January 24, 2013 11:16 AM
To: Verbeiren, David
Cc: qemu-devel@nongnu.org; aliguori@us.ibm.com
Subject: Re: [Qemu-devel] [PATCH] ui/vnc: VA API based H.264 encoding for VNC framebuffer updates

Hello Qemu people,

2013/1/9 David Verbeiren <david.verbeiren@intel.com>:
> This patch implements H.264 encoding of the VNC framebuffer updates 

> using hardware acceleration through the VA API.


Sounds Great !

Sorry if my question has a obvious response but I can't find it:
David, on which version of Qemu can we apply your attached patch ?

Thanks !

--
Jérôme Jutteau
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.