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

[Frediano Ziglio]

> 
> Hi Victor,
> 
> On Sat, 2016-10-15 at 15:38 +0200, Victor Toso wrote:
> > Hi,
> > 
> > On Thu, Sep 29, 2016 at 12:28:44PM +0100, Frediano Ziglio wrote:
> > > Small update for this patchset:
> > > - fix headers in "Extended protocol documentation";
> > > - added some more documentation on attributes.
> > > 
> > > Frediano Ziglio (4):
> > >   Start adding protocol file documentation
> > >   Start writing some documentation on protocol
> > >   Extended protocol documentation
> > >   More work on attribute protocol documentation
> > 
> > Ah, I thought this was in already, I was going to add some infom.
> > Let's have this, we can do more improvements or move it somewhere
> > else
> > if necessary later on.
> > 
> > Acked-by: Victor Toso <victortoso at redhat.com>
> > 
> > About what I was going to include is how the order of the messages
> > in
> > spice.proto need to be in the same order of what is declared in the
> > protocol itself otherwise the (de)marshalling could be using the
> > wrong
> > function to (un)serialize.
> > 
> > - full explanation just for reference
> > 
> > I'm playing with a new message to tell (or choose) the preferred
> > video
> > codecs for streams. So I've included a new enum.h like:
> > (note that message is after gl-draw-done here)
> 
> 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
> 
> Pavel

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.

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

> > 
> >  --- 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