[DVDnav-discuss] [PATCH] Add serialization support

[Paul Menzel]

Dear Richard,

Am Sonntag, den 20.10.2013, 19:10 +0200 schrieb Richard Hulme:
> On 18/10/13 23:20, Paul Menzel wrote:

> > Am Freitag, den 18.10.2013, 21:05 +0200 schrieb Richard Hulme:
> >
> >> The attached patch adds serialization support to libdvdnav, which
> >> effectively allows DVD bookmarks to be saved and restored.  The current
> >> VM state is converted to/from an ASCII string.
> >>
> >> This is a patch I created for MythTV, where it is currently in use.
> >>
> >> As I wrote in the submit description for MythTV:
> >>
> >> "The state snapshot code is borrowed/adapted from or inspired by XBMC
> >> and Ogle."
> >
> > thank you for your patch.
> >
> > Applying your patch to a git repository, committing it and doing `git
> > show` (with `git config color.ui auto` (default since 1.8.4)), it shows
> > several whitespace errors in `src/libdvdnav/Makefile`.

> As I forgot to add the changes to Makefile to the diff, any whitespace 
> errors there are not caused by me :)

Ah. So they are from Autotools(?) then I guess.

> > Furthermore, running
> >
> >          $ cppcheck --version # from Debian Sid/unstable
> >          Cppcheck 1.61
> >          $ cppcheck --enable=all src/vm/vm.c # Cppcheck 1.6
> >
> > does not show anything regarding your patch.
> 
> Is that good or bad?

That’s good.

> > Your usage of {}, if it is put on the same or on the next line, in
> > `src/dvdnav.c` seems inconsistent.
> 
> My usage of braces is consistent.  Other people's usage is sometimes 
> inconsistent with mine, which is unfortunate when I cut'n'paste their 
> code and don't think on to go over it all again :)
> 
> >> +  /* set the state. this will also start the vm on that state */
> >> +  /* means the next read block should be comming from that new */
> >> +  /* state */
> >
> > s/means/meaning/
> > s/comming/coming/
> 
> Yep, cut'n'paste.
> 
> Ok, I've made some changes (added the missing vm_serialize.c to 
> Makefile,

I think you need to add it to `src/vm/Makefile.am`. Though that did not
get it included into my main `Makefile` after running `./configure`.

> fixed formatting of braces, changed C++ style comments to C 
> and resolved some type issues).  Hopefully this looks a bit better.

Thanks for doing that!


Thanks,

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/attachments/20131021/53da67e5/attachment.asc>