[Spice-devel] [spice-common v1 2/4] messages: document limitation of id in StreamCreate

[Victor Toso]

Hi,

On Thu, Apr 19, 2018 at 03:31:08AM -0400, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > Note that the ID limitation always existed but now we have the
> > limitation in the protocol itself with SPICE_MAX_NUM_STREAMS
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  common/messages.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/common/messages.h b/common/messages.h
> > index b838881..4fc03d4 100644
> > --- a/common/messages.h
> > +++ b/common/messages.h
> > @@ -327,7 +327,7 @@ typedef struct SpiceMsgDisplayInvalOne {
> >  
> >  typedef struct SpiceMsgDisplayStreamCreate {
> >      uint32_t surface_id;
> > -    uint32_t id;
> > +    uint32_t id; /* Any number from 0 to SPICE_MAX_NUM_STREAMS - 1 */
> >      uint32_t flags;
> >      uint32_t codec_type;
> >      uint64_t stamp;
> 
> Was thinking something like
> 
> diff --git a/spice.proto b/spice.proto
> index 45c323f..c8a9656 100644
> --- a/spice.proto
> +++ b/spice.proto
> @@ -4,6 +4,11 @@
>  
>  typedef fixed28_4 int32 @ctype(SPICE_FIXED28_4);
>  
> +/* IDs of the video stream messages.
> + * These IDs are in the interval [0, SPICE_MAX_NUM_STREAMS)
> + */
> +typedef stream_id_t uint32;
> +
>  struct Point {
>      int32 x;
>      int32 y;
> @@ -699,7 +704,7 @@ struct String {
>  };
>  
>  struct StreamDataHeader {
> -	uint32 id;
> +	stream_id_t id;
>  	uint32 multi_media_time;
>  };
>  
> @@ -757,7 +762,7 @@ channel DisplayChannel : BaseChannel {
>  
>      message {
>  	uint32 surface_id;
> -	uint32 id;
> +	stream_id_t id;
>  	stream_flags flags;
>  	video_codec_type codec_type;
>  	uint64 stamp;
> @@ -776,12 +781,12 @@ channel DisplayChannel : BaseChannel {
>      } stream_data;
>  
>      message {
> -	uint32 id;
> +	stream_id_t id;
>  	Clip clip;
>      } stream_clip;
>  
>      message {
> -	uint32 id;
> +	stream_id_t id;
>      } stream_destroy;
>  
>      Empty stream_destroy_all;
> @@ -964,7 +969,7 @@ channel DisplayChannel : BaseChannel {
>      } draw_composite;
>  
>      message {
> -        uint32 stream_id;
> +        stream_id_t stream_id;
>          uint32 unique_id;
>          uint32 max_window_size;
>          uint32 timeout_ms;
> @@ -996,7 +1001,7 @@ channel DisplayChannel : BaseChannel {
>      } init = 101;
>  
>      message {
> -        uint32 stream_id;
> +        stream_id_t stream_id;
>          uint32 unique_id;
>          // the mm_time of the first frame included in the report
>          uint32 start_frame_mm_time;
> 
> 
> not sure about typedef naming, we don't use this feature that
> much

Yes, we don't do that much but we could start where it makes
sense, like here.

I did not test this, doesn't it change the marshallers code? If
no drawbacks, I'm all for more explicit types in the protocol.

Thanks,
        toso
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180419/88fd26b6/attachment.sig>