Re: [Dvd-devel] Forking of libdvdread

Fri May 09 2003, James Courtier-Dutton wrote:
> Håkan Hjort wrote:
> > Wed May 07 2003, Michael Roitzsch wrote:
> >>From what James explained to me:
> >>He thinks the bitfield structures are very insecure concerning 
> >>portability, since compilers are known to "optimize" such packed data 
> >>and align it in some way. I agree with that.
> >>IIRC you already had such problems with some Sun compiler and fixed some 
> >>in a rather hacky way by changing the order of one of the structures to 
> >>one the compiler does handle.
> >>James thinks that a portable way would be to use a getbits() method to 
> >>read the data from the disc and store it in the structures. I think he 
> >>is right here. He said he discussed that with the libdvdread team 
> >>(you?), but they denied that need.
> >>
> >I simply said that _I_ would not be making those changes.  I have
> >no problem with patches making libdvdread more portable, though 
> >this would again break the ABI.
> >
> >More specifically to 'getbits()', I feel that that should not be
> >necessary, the IFO and NAV structures are almost exlucivly byte /
> >short / word length data and at fixed offsets from some point that can
> >be computed.  So I'd think it would be better to make it byte based,
> >and only in those few cases where there are bit based fileds handle
> >those with a bits based macro/function.  Though this is just details.
> >
> 
> If you care to look at my getbits() function, you will in fact see that 
> it is dual bit/byte ready.
> I am using the getbits() function to parse the binary packed NAV and IFO 
> structures into C structures.
> This parsing only happens once in a while, so performance is certainly 
> not the issue here.
> I prefer nicer looking code in this case.

Yes, I'm not sure that using bits would make nice code... since the
structs are infact byte packed with few exceptions, the hilight info
in the NAV packets beeing one, but anyway... not the most important
issue here.

> >>>>libdvdread does not like Microsoft Compilers.
> >>>>       
> >>>>
> >>>Yes, so why do you need to use them?  There are the mingwin and
> >>>cygwin systems for Win32 platforms.  Granted I've not tested with
> >>>them either.
> >>>     
> >>>
> >>That's not the point here. MSVC is not a very good compiler, but a very 
> >>widely spread one. libdvdread should compile with it if open-source 
> >>DVD-playback wants to conquer the Windows platform.
> >>
Agaian I don't see why, the users won't compile it them self anyway...
but if you say so.

> >Oh well.. I still feel that mingwin could be used instead
> >(if that isn't broken too).
> >
> The current way libdvdread does parsing or mapping from ifo and nav is 
> the wrong way to do it.
> Packing of bit field structs in the C language will vary from compiler 
> to compiler and even from compiler versions to other versions, so to 
> have truely portable C code, one cannot assume any particular packing 
> format. It was this issue that I was striving to fix and I believe I 
> have managed that with some fairly neat looking code. Currently, only 
> nav parsing and header files have changed.
> 
So it's wrong, but it works. It's indeed more work to do it 100%
portable. Let's drop this, I'll incorporate any patches that makes
libdvdread more portable as long as they are reasonably maintainable.

-snip-

> Maybe I should try to explain some of the reasons for doing this.
> 1) I wanted to get libdvdnav and associated tools to compile on any 
> platform. I believe the getbits() and possibly adding get16/32bits() 
> achieve this.

Code that up, send patches.  We're just an email away. 

> 2) I wanted to experiment with different methods of error reporting.

Fine, this is something that needs improvement, patches welcome.

>  a) The application to get knowledge if the DVD required 
> authentication/ diagnose errors. E.g. DVD drive region locking etc.

This isn't done becuse it's libdvdcss that handles this, and it has a
very minimal interface.  libdvdcss needs to be changed befor anything
can be done in libdvdread.  I've been intrested in this also but 
short of a expansion of the libdvdcss interface I don't see what can
be done.

>  b) The application to get knowledge if the DVD requires CSS support.

Same here, it's mainly a libdvdcss problem.

>  c) The application to handle errors on the DVD sector reads.

I'm open to sugestions.  What do you think is wrong currently?


> 3) Lots of other projects currently use libdvdread, and any changes we 
> might make to the api would take a long time to reach the user. A lot of 
> package handlers do not like 2 versions of the same lib being loaded, so 
> upgrading libdvdread on the user's platform would probably break other 
> applications.

I don't see what you are saying here, it's widely used so it would
take a long time to make changes?  It beeing widely used should be a
good argument for continuing development, not to fork it.
Crappy package handlers should not be a hindrance, see this as
encuragement for those people that are responsible for them to fix
such short commings.  Do you know of any specific ones with 
such problems?

> 4) libdvdnav is still very much a work in progress, particularly with 
> regard to the api, so making changes to the current libdvdread 
> functionallity, if needed, could be made in the libdvdnav tree without 
> effecting libdvdread in anyway.

What do you mean?
a) libdvdnav changes often
b) copying the files from libdvdread will let you change them often too  
c) all this will result in no changes to libdvdread.

Yes that is true, but point c here is exactly the problem, no
changes will come back to libdvdread.


> 5) I could not really see why 2 different libs were really technically 
> needed. Having one lib seemed to me to be neater.

Perhaps so, the library is already there however and my feeling is
that not everything needs to be made in to one monolitical lib.  I see
more reason for libdvdread and libdvdcss to be merged than for this.

> 6) libdvdread changes very rarely, so syncing libdvdread changes into 
> libdvdnav will not be a big problem.

Ah, but that's not the way, changes should be merged into libdvdread,
not from.

> 7) I see this more of a re-use of some source code files, rather than a 
> fork.

There is a library that exports the functionality, so why make copies of
the files?  They will just diverge and each will end up it's own set of
bugs and shortcomings.

My feeling is that it's much better to improve libdvdread than to copy
this code and then at some later point perhaps go over it and have to
try to extract things to put back in.  This will just result in more
work, work that might not get done at all.


> 8) The videolan project have their own dvd lib, libdvdplay, and I think 
> that will be doing it's own dvd reading, and ifo/nav parsing. I can't 
> really understand their need for it, when libdvdnav is already here, but 
> they fail to answer any emails, so I don't really know why they have 
> embarked on it.
> 
I can't really answer that for you.


-- 
Håkan Hjort