[Spice-devel] [PATCH v2 0/4] Protocol file syntax documentation

[Victor Toso]

Hi,

> > Not sure if I understand you correctly - but you should not modify
> > enums.h by yourself. As the first line of the file says, it should
> > be generated by spice_codegen.py (spice-common repository), eg:
> >
> > ./spice_codegen.py -e spice.proto ../spice-protocol/spice/enums.h

So, that should go in the documentation. I did not know that.

> If I understood correctly what Victor is trying to say is that in
> the channel definition the order of the messages is important as
> messages are associated to numbers more or less in a similar way to
> enum in C. This should go in the TODO under the "Channels" section.

The messages id are given by spice-protocol/spice/enums.h but the
spice-common marshalling code ignores its order as it is generated by
what is defined in spice.proto;

For me, it is really odd that the common code between spice and
spice-gtk dictates spice-protocol definitions. From my initial
perspective, we first define the protocol and then implement
(spice.proto for the serialization code and then the actual handlers in
clint, server and/or agent);

As we have start changing spice.proto to define the spice/enums.h
header, I think we should move spice.proto to spice-protocol code.

>
> OT: looking at file seems that there should be a section for
> protocol specification too.

Indeed :)

Cheers,
  toso

>
> > > 
> > >  --- a/spice/enums.h
> > >  +++ b/spice/enums.h
> > >  @@ -529,6 +529,7 @@ enum {
> > >      SPICE_MSGC_DISPLAY_STREAM_REPORT,
> > >      SPICE_MSGC_DISPLAY_PREFERRED_COMPRESSION,
> > >      SPICE_MSGC_DISPLAY_GL_DRAW_DONE,
> > >  +   SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE,
> > > 
> > >      SPICE_MSGC_END_DISPLAY
> > >  };
> > > 
> > > But what I did on spice.proto was:
> > > (the message is before gl-draw-done)
> > > 
> > >  --- a/spice.proto
> > >  +++ b/spice.proto
> > >  @@ -984,6 +984,10 @@ channel DisplayChannel : BaseChannel {
> > >       } preferred_compression;
> > > 
> > >        message {
> > >  +        video_codec_type codec_type;
> > >  +    } preferred_video_codec_type;
> > >  +
> > >  +    message {
> > >       } gl_draw_done;
> > >   };
> > > 
> > > The outcome in common/generated_server_demarshallers.c is
> > > 
> > > (...)
> > >  static parse_msg_func_t funcs2[5] =  {
> > >      parse_msgc_display_init,
> > >      parse_msgc_display_stream_report,
> > >      parse_msgc_display_preferred_compression,
> > >      parse_msgc_display_preferred_video_codec_type
> > >      parse_msgc_display_gl_draw_done,
> > >  };
> > > 
> > > The handlers follow the order of spice.proto, meaning that on
> > > demarshalling it was using parse_msgc_display_gl_draw_done()
> > > function
> > > for SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE message.
> > > 
> > > ;)
> > > 
> > > Cheers,
> > >   toso
> > > 
> > > > 
> > > >  Makefile.am             |   2 +-
> > > >  configure.ac            |   2 +
> > > >  docs/Makefile.am        |  17 ++
> > > >  docs/spice_protocol.txt | 423
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  m4/spice_manual.m4      |  32 ++++
> > > >  5 files changed, 475 insertions(+), 1 deletion(-)
> > > >  create mode 100644 docs/Makefile.am
> > > >  create mode 100644 docs/spice_protocol.txt
> > > >  create mode 100644 m4/spice_manual.m4
> > > > 
> 
> Frediano
-------------- 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/20161017/13b0de62/attachment.sig>