[DVDnav-discuss] VLC patches

[Damien Fouilleul]

just a few comments on top of j-b said


> >/ > Some of this patch is to compile correctly under some version of/
> >/ > cygwin/MSys (lseek, gettimeoftheday and LL)/
> >/ /
> >/ it's ugly and especially out of place: who wrote that part/
> >/ should have fixed cygwin instead/
> ...
> Cygwin/MSys being not perfect is known, but telling people to fix their
> environment is not really a solution.

> > Mingw runtime since 3.10 has gettimeofday, so the first configure.ac
> > patch is needed to detect it.

> >/ why? doesn't cygwin have dlopen()?
/> the libdvdcss part, you can of course not use it...

one more thing, those patches are for mingw, not cygwin. they are quite different, 
mingw is about using windows APIs natively and as such you can't expect to see POSIX compliance,
but mingw does provide some POSIX apis that are not part of windows SDK, and each release of mingw
contains more of these.

we could use the windows equivalent for dlopen(), which is called LoadLibrary() by the way, but VLC for win32
use static archives anyway. maybe in the future. 

windows does not support the _FILE_OFFSET_BITS=64 kludge, instead it has its own apis called _lseeki64, etc...

> >/ > And the last one is to use dvdinput_open(device) instead of
/> >/ > open(device, O_RDONLY);
/> >/ 
/> >/ why?
/> On win32, you can't open a block device.

well, technically you can, but not from a drive letter, which needs be converted to a device path, and that only works on win2000+
Anyway, libdvdread already supports reading block devices on win32, so why not reuse it, and it's compatible with all platforms

> >/ > wanted to inform you about that.
/> >/ the patch to bswap.h seems to be essentially cosmetical
/> Yes, but fixes some weird cygwin/win64 problems, IIRC.

well, it's not quite cosmetic, if you look at the following line

      (((x) & 0x00000000000000ff) << 56))

in order to type the constant, the C compiler usually uses the type with the most convenient storage size that can accommodate it,
which in this case would be an int. But it would also consider the type of x, and use its type to evaluate the expression it if its storage size is bigger than a int.
therefore if you 100% sure that x would always be an int64_t, then you will never have a problem, but if you use another type such as in32_t, etc..
then <<56 would overflow and cause a problem, by using 0x00000000000000ffLL, you force the compiler to use int64_t regardless of the type of x.

> >/ The diff in dvd_input.c:
/> >/ - for(i=25; i < 73; i++ ) { 
/> >/ +  for(i=40; i < 73; i++ ) { 
/> >/ is unexplainable. Can you shed some light?
/> This diff is not in dvd_input.c but in vm.c and is realted to the use of
> dvdinput_open.
> I can ask more to damien.

i got those offsets from libdvdcss (libdvdcss.c:431), and they do match what you physically see on the DVD.

Damien