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

[Frediano Ziglio]

> 
> On Tue, 2017-01-31 at 12:08 +0100, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> > 
> > Similar to preferred video compression, a radio button showing
> > mjpeg,
> > vp8 and h264 in case server has the proper [0] capability
> 
> Please update for the recently added vp9
> 

Would be worth also reusing the array you introduced with
the list of all codecs instead of adding them here and there.
For instance the menu list should/could be generated from
it.

> > 
> > [0] SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  tools/spicy.c | 56
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> > 
> > diff --git a/tools/spicy.c b/tools/spicy.c
> > index c502428..bffbef8 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,22 @@ static const GtkRadioActionEntry
> > compression_entries[] = {
> >      }
> >  };
> >  
> > +static const GtkRadioActionEntry video_codec_type_entries[] = {
> > +    {
> > +        .name  = "mjpeg",
> > +        .label = "mjpeg",
> > +        .value = SPICE_VIDEO_CODEC_TYPE_MJPEG,
> > +    },{
> > +        .name  = "vp8",
> > +        .label = "vp8",
> > +        .value = SPICE_VIDEO_CODEC_TYPE_VP8,
> > +    },{
> > +        .name  = "h264",
> > +        .label = "h264",
> > +        .value = SPICE_VIDEO_CODEC_TYPE_H264,
> > +    }
> > +};
> > +
> >  static char ui_xml[] =
> >  "<ui>\n"
> >  "  <menubar action='MainMenu'>\n"
> > @@ -864,6 +883,11 @@ 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='h264'/>\n"
> > +"      </menu>\n"
> >  "    </menu>\n"
> >  "    <menu action='HelpMenu'>\n"
> >  "      <menuitem action='About'/>\n"
> > @@ -916,6 +940,14 @@ static void compression_cb(GtkRadioAction
> > *action G_GNUC_UNUSED,
> >                                                 gtk_radio_action_get
> > _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_actio
> > n_get_current_value(current));
> > +}
> > +
> >  static void
> >  spice_window_class_init (SpiceWindowClass *klass)
> >  {
> > @@ -970,6 +1002,30 @@ 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,
> OT:
> "gtk_action_group_add_radio_actions has been deprecated since version
> 3.10 and should not be used in newly-written code."
> 
> Can we do something about it ?
> 
> > video_codec_type_entries,
> > +                                       G_N_ELEMENTS(video_codec_typ
> > e_entries), -1,
> > +                                       G_CALLBACK(video_codec_type_
> > cb), win->display_channel);
> > +    if (!spice_channel_test_capability(win->display_channel,
> > +                                       SPICE_DISPLAY_CAP_PREF_VIDEO
> > _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 {
> > +        GtkAction *submenu_action;
> > +        if (!spice_channel_test_capability(win->display_channel,
> > SPICE_DISPLAY_CAP_CODEC_MJPEG)) {
> > +            submenu_action = gtk_action_group_get_action(win->ag,
> > "mjpeg");
> > +            gtk_action_set_sensitive(submenu_action, TRUE);
> > +        }
> > +        if (!spice_channel_test_capability(win->display_channel,
> > SPICE_DISPLAY_CAP_CODEC_VP8)) {
> > +            submenu_action = gtk_action_group_get_action(win->ag,
> > "vp8");
> > +            gtk_action_set_sensitive(submenu_action, TRUE);
> > +        }
> > +        if (!spice_channel_test_capability(win->display_channel,
> > SPICE_DISPLAY_CAP_CODEC_H264)) {
> > +            submenu_action = gtk_action_group_get_action(win->ag,
> > "h264");
> > +            gtk_action_set_sensitive(submenu_action, TRUE);
> > +        }
> 
> Please avoid c&p
> 
> > +    }
> > +
> >      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));
> 
> Looks good otherwise,
> Pavel
> 

Frediano