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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> > > 
> > > 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@xxxxxxxxxx>
> > > ---
> > >  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));
> > 
> > 

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]