[Spice-devel] [spice-common v3 02/11] protocol: add preferred video codec message

[Frediano Ziglio]

> 
> Hi,
> 
> On Wed, Dec 14, 2016 at 02:48:45PM -0500, Frediano Ziglio wrote:
> > > 
> > > From: Victor Toso <me at victortoso.com>
> > > 
> > > Client might want to choose a preferred video codec for streaming for
> > > different reasons which having hardware decoder support being the most
> > > interest one.
> > > 
> > > This message allows the client to send an array of video codecs in
> > > order of preference.
> > > 
> > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > ---
> > >  common/messages.h | 5 +++++
> > >  spice.proto       | 5 +++++
> > >  2 files changed, 10 insertions(+)
> > > 
> > > diff --git a/common/messages.h b/common/messages.h
> > > index 516a345..5316cc0 100644
> > > --- a/common/messages.h
> > > +++ b/common/messages.h
> > > @@ -644,6 +644,11 @@ typedef struct SpiceMsgcPortEvent {
> > >      uint8_t event;
> > >  } SpiceMsgcPortEvent;
> > >  
> > > +typedef struct SpiceMsgcDisplayPreferredVideoCodecType {
> > > +    uint32_t num_of_codecs;
> > > +    uint8_t codecs[0];
> > 
> > This is weird the type of codecs[0] suggest there can be only up
> > to 256 codecs while num_of_codecs type suggest there can be
> > much more. And taking into account that the enumeration starts
> > with 1 even specifying the full 1-255 range can't take more
> > than 255 items.
> 
> True! Having num_of_codecs as uint8_t is enough? Supporting 255 codecs
> would be incredible :)
> 

Yes, I think uint8 in the protocol file would be better.

> > 
> > > +} SpiceMsgcDisplayPreferredVideoCodecType;
> > > +
> > >  typedef struct SpiceMsgcDisplayPreferredCompression {
> > >      uint8_t image_compression;
> > >  } SpiceMsgcDisplayPreferredCompression;
> > > diff --git a/spice.proto b/spice.proto
> > > index 0bfc515..758f9bb 100644
> > > --- a/spice.proto
> > > +++ b/spice.proto
> > > @@ -985,6 +985,11 @@ channel DisplayChannel : BaseChannel {
> > >  
> > >      message {
> > >      } gl_draw_done;
> > > +
> > > +    message {
> > > +        uint32 num_of_codecs;
> > > +        video_codec_type codecs[num_of_codecs] @end;
> > > +    } preferred_video_codec_type;
> > >  };
> > >  
> > >  flags16 keyboard_modifier_flags {
> > 

Frediano