[Spice-devel] [spice-gtk v7 3/3] spicy: implement preferred video codec type

Victor Toso victortoso at redhat.com
Mon Feb 6 20:52:56 UTC 2017


Hi,

On Mon, Feb 06, 2017 at 06:32:21PM +0100, Pavel Grunt wrote:
> On Mon, 2017-02-06 at 17:52 +0100, Pavel Grunt wrote:
> > On Mon, 2017-02-06 at 12:06 +0100, Victor Toso wrote:
> > > From: Victor Toso <me at victortoso.com>
> > > 
> > > Similar to preferred video compression, a radio button showing
> > > mjpeg,
> > > vp8, vp9 and h264 in case server has the proper [0] capability
> > > 
> > > [0] SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE
> > > 
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > ---
> > >  tools/spicy.c | 64
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 64 insertions(+)
> > > 
> > > diff --git a/tools/spicy.c b/tools/spicy.c
> > > index c502428..ce758a7 100644
> > > --- a/tools/spicy.c
> > > +++ b/tools/spicy.c
> > > @@ -651,6 +651,9 @@ static const GtkActionEntry entries[] = {
> > >          .name        = "CompressionMenu",
> > >          .label       = "_Preferred image compression",
> > >      },{
> > > +        .name        = "VideoCodecTypeMenu",
> > > +        .label       = "_Preferred video codec type",
> > > +    },{
> > >          .name        = "HelpMenu",
> > >          .label       = "_Help",
> > >      },{
> > > @@ -816,6 +819,26 @@ static const GtkRadioActionEntry
> > > compression_entries[] = {
> > >      }
> > >  };
> > >  
> > > +static const GtkRadioActionEntry video_codec_type_entries[] = {
> > > +    {
> > > +        .name  = "mjpeg",
> > > +        .label = "mjpeg",
> > > +        .value = SPICE_VIDEO_CODEC_TYPE_MJPEG,
> > 
> > if we had some structure like Frediano suggested, we could reuse it
> > here
> > > +    },{
> > > +        .name  = "vp8",
> > > +        .label = "vp8",
> > > +        .value = SPICE_VIDEO_CODEC_TYPE_VP8,
> > > +    },{
> > > +        .name  = "vp9",
> > > +        .label = "vp9",
> > > +        .value = SPICE_VIDEO_CODEC_TYPE_VP9,
> > > +    },{
> > > +        .name  = "h264",
> > > +        .label = "h264",
> > > +        .value = SPICE_VIDEO_CODEC_TYPE_H264,
> > > +    }
> > > +};
> > > +
> > >  static char ui_xml[] =
> > >  "<ui>\n"
> > >  "  <menubar action='MainMenu'>\n"
> > > @@ -864,6 +887,12 @@ static char ui_xml[] =
> > >  #endif
> > >  "        <menuitem action='off'/>\n"
> > >  "      </menu>\n"
> > > +"      <menu action='VideoCodecTypeMenu'>\n"
> > > +"        <menuitem action='mjpeg'/>\n"
> > > +"        <menuitem action='vp8'/>\n"
> > > +"        <menuitem action='vp9'/>\n"
> > > +"        <menuitem action='h264'/>\n"
> > > +"      </menu>\n"
> > >  "    </menu>\n"
> > >  "    <menu action='HelpMenu'>\n"
> > >  "      <menuitem action='About'/>\n"
> > > @@ -916,6 +945,14 @@ static void compression_cb(GtkRadioAction
> > > *action G_GNUC_UNUSED,
> > >                                                 gtk_radio_action_g
> > > et
> > > _current_value(current));
> > >  }
> > >  
> > > +static void video_codec_type_cb(GtkRadioAction *action
> > > G_GNUC_UNUSED,
> > > +                                GtkRadioAction *current,
> > > +                                gpointer user_data)
> > > +{
> > > +    spice_display_change_preferred_video_codec_type(SPICE_CHANNEL
> > > (u
> > > ser_data),
> > > +                                                    gtk_radio_act
> > > io
> > > n_get_current_value(current));
> > > +}
> > > +
> > >  static void
> > >  spice_window_class_init (SpiceWindowClass *klass)
> > >  {
> > > @@ -970,6 +1007,33 @@ static SpiceWindow
> > > *create_spice_window(spice_connection *conn, SpiceChannel *ch
> > >          GtkAction *compression_menu_action =
> > > gtk_action_group_get_action(win->ag, "CompressionMenu");
> > >          gtk_action_set_sensitive(compression_menu_action, FALSE);
> > >      }
> > > +    gtk_action_group_add_radio_actions(win->ag,
> > > video_codec_type_entries,
> > > +                                       G_N_ELEMENTS(video_codec_t
> > > yp
> > > e_entries), -1,
> > > +                                       G_CALLBACK(video_codec_typ
> > > e_
> > > cb), win->display_channel);
> > > +    if (!spice_channel_test_capability(win->display_channel,
> > > +                                       SPICE_DISPLAY_CAP_PREF_VID
> > > EO
> > > _CODEC_TYPE)) {
> > > +        GtkAction *video_codec_type_menu_action =
> > > +            gtk_action_group_get_action(win->ag,
> > > "VideoCodecTypeMenu");
> > > +        gtk_action_set_sensitive(video_codec_type_menu_action,
> > > FALSE);
> > > +    } else {
> > > +        gint i;
> > 
> > it should be unsigne
> > > +        static const struct {
> > > +            gint cap;
> > > +            const gchar name[8];
> > > +        } display_codecs[] = {
> > > +            {SPICE_DISPLAY_CAP_CODEC_MJPEG, "mjpeg"},
> > > +            {SPICE_DISPLAY_CAP_CODEC_VP8, "vp8"},
> > > +            {SPICE_DISPLAY_CAP_CODEC_H264, "h264"},
> > > +            {SPICE_DISPLAY_CAP_CODEC_VP9, "vp9"},
> > > +        };
> > > +        for (i = 0; i < G_N_ELEMENTS(display_codecs); i++) {
> > > +            if (!spice_channel_test_capability(win-
> > > > display_channel, display_codecs[i].cap)) {
> > 
> > isn't the condition wrong ?
> > 
> > The menu is gray all the time for me...
> the gray menu is another issue, the problem is that
> spice_channel_test_capability() tests remote caps (c->remote_caps) but
> spicy is interested in local caps (c->caps)

Aha. It should check both, remote and local caps. Nice catch :)

Also, it seems that spice does not check the encoding elements like
spice-gtk does with the decoding ones. I've manually removed from the
path vp8 & vp9 plugins and spicy submenu does not goes gray. Spice-gtk
knows that it can't find the elements...

I'll try to fix spice in a separated patch and improve this check in
spicy.

Let me know if there is anything else in the rest of the patches as this
changes are becoming minor. Once again, thanks for the review.

> 
> > 
> > Pavel
> > 
> > > +                GtkAction *action =
> > > gtk_action_group_get_action(win->ag, display_codecs[i].name);
> > > +                gtk_action_set_sensitive(action, TRUE);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > >      gtk_ui_manager_insert_action_group(win->ui, win->ag, 0);
> > >      gtk_window_add_accel_group(GTK_WINDOW(win->toplevel),
> > >                                 gtk_ui_manager_get_accel_group(win
> > > -
> > > > ui));
> > 
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170206/0690eaad/attachment-0001.sig>


More information about the Spice-devel mailing list