Re: [PATCH v5] Add support for LED state extension to gvnc

[Lei Li <lilei linux vnet ibm com>]

Hi Daniel,

Do you think this patch is ready to be merged?

On 05/09/2013 10:29 PM, Lei Li wrote:
> Hi guys,
>
> As we are trying to add LED state extension to VNC protocol, and
> the implementation for Qemu VNC server has already upstream. We'd
> like to send out this patch, which tries to add support to gvnc,
> please let me know if there is anything need to be addressed.
>
> The proposal and patch series for detail:
>
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04990.html
>
> Thanks!
>
> Changes since v4:
>    Make the constants statisfy naming convention suggested by Daniel.
>
> Changes since v3:
>    Fix the wrong naming of constants.
>
> Changes since v2:
>    Address comments from Daniel includes:
>      - Include ledstate related constants.
>      - Indentation fix.
>      - Add new callback vnc_led_state() at the end, and remove the
>        corresponding amount of padding.
>
> Changes since v1:
>    Add signal callback vnc_led_state().
>
> Signed-off-by: Lei Li <lilei linux vnet ibm com>
> ---
>   src/libgvnc_sym.version |    1 +
>   src/vncconnection.c     |   44 ++++++++++++++++++++++++++++++++++++++++++++
>   src/vncconnection.h     |   10 +++++++++-
>   3 files changed, 54 insertions(+), 1 deletions(-)
>
> diff --git a/src/libgvnc_sym.version b/src/libgvnc_sym.version
> index 9adec25..d029a81 100644
> --- a/src/libgvnc_sym.version
> +++ b/src/libgvnc_sym.version
> @@ -87,6 +87,7 @@
>         vnc_connection_set_audio_format;
>         vnc_connection_get_audio_format;
>         vnc_connection_set_audio;
> +       vnc_connection_get_ledstate;
>
>         vnc_util_set_debug;
>         vnc_util_get_debug;
> diff --git a/src/vncconnection.c b/src/vncconnection.c
> index 4b25a96..569f441 100644
> --- a/src/vncconnection.c
> +++ b/src/vncconnection.c
> @@ -217,6 +217,7 @@ struct _VncConnectionPrivate
>       guint8 zrle_pi;
>       int zrle_pi_bits;
>
> +    int ledstate;
>       gboolean has_ext_key_event;
>
>       struct {
> @@ -248,6 +249,7 @@ enum {
>       VNC_FRAMEBUFFER_UPDATE,
>       VNC_DESKTOP_RESIZE,
>       VNC_PIXEL_FORMAT_CHANGED,
> +    VNC_LED_STATE,
>
>       VNC_AUTH_FAILURE,
>       VNC_AUTH_UNSUPPORTED,
> @@ -449,6 +451,7 @@ struct signal_data
>           VncCursor *cursor;
>           gboolean absPointer;
>           const char *text;
> +        int ledstate;
>           struct {
>               int x;
>               int y;
> @@ -525,6 +528,13 @@ static gboolean do_vnc_connection_emit_main_context(gpointer opaque)
>                         data->params.pixelFormat);
>           break;
>
> +    case VNC_LED_STATE:
> +        g_signal_emit(G_OBJECT(data->conn),
> +                      signals[data->signum],
> +                      0,
> +                      data->params.ledstate);
> +        break;
> +
>       case VNC_AUTH_FAILURE:
>           g_signal_emit(G_OBJECT(data->conn),
>                         signals[data->signum],
> @@ -1415,6 +1425,19 @@ static void vnc_connection_read_pixel_format(VncConnection *conn, VncPixelFormat
>                 fmt->red_shift, fmt->green_shift, fmt->blue_shift);
>   }
>
> +static void vnc_connection_ledstate_change(VncConnection *conn)
> +{
> +    VncConnectionPrivate *priv = conn->priv;
> +    struct signal_data sigdata;
> +
> +    priv->ledstate = vnc_connection_read_u8(conn);
> +
> +    VNC_DEBUG("LED state: %d\n", priv->ledstate);
> +
> +    sigdata.params.ledstate = priv->ledstate;
> +    vnc_connection_emit_main_context(conn, VNC_LED_STATE, &sigdata);
> +}
> +
>   /* initialize function */
>
>   gboolean vnc_connection_has_error(VncConnection *conn)
> @@ -2884,6 +2907,10 @@ static gboolean vnc_connection_framebuffer_update(VncConnection *conn, gint32 et
>           vnc_connection_pointer_type_change(conn, x);
>           vnc_connection_resend_framebuffer_update_request(conn);
>           break;
> +    case VNC_CONNECTION_ENCODING_LED_STATE:
> +        vnc_connection_ledstate_change(conn);
> +        vnc_connection_resend_framebuffer_update_request(conn);
> +        break;
>       case VNC_CONNECTION_ENCODING_WMVi:
>           vnc_connection_read_pixel_format(conn, &priv->fmt);
>           vnc_connection_pixel_format(conn);
> @@ -4609,6 +4636,16 @@ static void vnc_connection_class_init(VncConnectionClass *klass)
>                         1,
>                         G_TYPE_POINTER);
>
> +    signals[VNC_LED_STATE] =
> +        g_signal_new ("vnc-led-state",
> +                      G_OBJECT_CLASS_TYPE (object_class),
> +                      G_SIGNAL_RUN_FIRST,
> +                      G_STRUCT_OFFSET (VncConnectionClass, vnc_led_state),
> +                      NULL, NULL,
> +                      g_cclosure_marshal_VOID__INT,
> +                      G_TYPE_NONE,
> +                      0);
> +
>       signals[VNC_AUTH_FAILURE] =
>           g_signal_new ("vnc-auth-failure",
>                         G_OBJECT_CLASS_TYPE (object_class),
> @@ -5444,6 +5481,13 @@ gboolean vnc_connection_get_abs_pointer(VncConnection *conn)
>       return priv->absPointer;
>   }
>
> +int vnc_connection_get_ledstate(VncConnection *conn)
> +{
> +    VncConnectionPrivate *priv = conn->priv;
> +
> +    return priv->ledstate;
> +}
> +
>   /*
>    * Local variables:
>    *  c-indent-level: 4
> diff --git a/src/vncconnection.h b/src/vncconnection.h
> index cbfefd7..72d3f38 100644
> --- a/src/vncconnection.h
> +++ b/src/vncconnection.h
> @@ -39,6 +39,11 @@ G_BEGIN_DECLS
>   #define VNC_IS_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), VNC_TYPE_CONNECTION))
>   #define VNC_CONNECTION_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), VNC_TYPE_CONNECTION, 
> VncConnectionClass))
>
> +/* identical to the ledstate bits */
> +#define VNC_LEDSTATE_SCROLL_LOCK (1 << 0)
> +#define VNC_LEDSTATE_NUM_LOCK    (1 << 1)
> +#define VNC_LEDSTATE_CAPS_LOCK   (1 << 2)
> +
>
>   typedef struct _VncConnection VncConnection;
>   typedef struct _VncConnectionPrivate VncConnectionPrivate;
> @@ -73,12 +78,13 @@ struct _VncConnectionClass
>       void (*vnc_connected)(VncConnection *conn);
>       void (*vnc_initialized)(VncConnection *conn);
>       void (*vnc_disconnected)(VncConnection *conn);
> +    void (*vnc_led_state)(VncConnection *conn);
>
>       /*
>        * If adding fields to this struct, remove corresponding
>        * amount of padding to avoid changing overall struct size
>        */
> -    gpointer _vnc_reserved[VNC_PADDING_LARGE];
> +    gpointer _vnc_reserved[VNC_PADDING_LARGE - 4];
>   };
>
>
> @@ -114,6 +120,7 @@ typedef enum {
>       VNC_CONNECTION_ENCODING_POINTER_CHANGE = -257,
>       VNC_CONNECTION_ENCODING_EXT_KEY_EVENT = -258,
>       VNC_CONNECTION_ENCODING_AUDIO = -259,
> +    VNC_CONNECTION_ENCODING_LED_STATE = -261,
>   } VncConnectionEncoding;
>
>   typedef enum {
> @@ -203,6 +210,7 @@ VncCursor *vnc_connection_get_cursor(VncConnection *conn);
>
>   gboolean vnc_connection_get_abs_pointer(VncConnection *conn);
>   gboolean vnc_connection_get_ext_key_event(VncConnection *conn);
> +int vnc_connection_get_ledstate(VncConnection *conn);
>
>   gboolean vnc_connection_set_audio(VncConnection *conn,
>                                     VncAudio *audio);


--
Lei