Discussion:
[libdvdcss-devel] [PATCH] Make dependency on direct.h explicit
Alexander Strasser
2013-01-29 20:15:14 UTC
Permalink
Hi all!

I attached a patch to fix libdvdcss build on OS/2
from inside MPlayer source directory.

I also had the patch removing the direct.h entirely
but Diego sent his version earlier so I only append
the patch mentioned in the subject.

A commit message is also contained in the patch file.

I tested on MinGW32, Cygwin and Linux. I think it is
also safe for MSVC. Testing would be welcome of course.

I did the same for the other patch that is equivalent
to Diego's. I am not sure it will work with MSVC though.

Testing on OS/2 would also be welcome just to be safe.

Alexander
Diego Biurrun
2013-01-29 20:23:42 UTC
Permalink
Post by Alexander Strasser
I attached a patch to fix libdvdcss build on OS/2
from inside MPlayer source directory.
--- src/libdvdcss.c
+++ src/libdvdcss.c
@@ -120,7 +120,7 @@
-#ifdef HAVE_DIRECT_H
+#if defined(WIN32) && defined(HAVE_DIRECT_H) && HAVE_DIRECT_H
# include <direct.h>
#endif
@@ -238,7 +238,7 @@ LIBDVDCSS_EXPORT dvdcss_t dvdcss_open ( char *psz_target )
if( psz_cache == NULL || psz_cache[0] == '\0' )
{
-#ifdef HAVE_DIRECT_H
+#if defined(WIN32) && defined(HAVE_DIRECT_H) && HAVE_DIRECT_H
typedef HRESULT( WINAPI *SHGETFOLDERPATH )
( HWND, int, HANDLE, DWORD, LPTSTR );
These are MPlayer-specific hacks that have no place in libdvdcss.

Diego
Jean-Baptiste Kempf
2013-01-30 17:31:41 UTC
Permalink
Post by Alexander Strasser
I attached a patch to fix libdvdcss build on OS/2
from inside MPlayer source directory.
This patch is even more wrong than the previous ones: where are
direct.h functions being used on this chunk of code?

Best regards,
--
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device
Alexander Strasser
2013-01-30 18:18:28 UTC
Permalink
Hi Jean-Baptiste!
Post by Jean-Baptiste Kempf
Post by Alexander Strasser
I attached a patch to fix libdvdcss build on OS/2
from inside MPlayer source directory.
I am not sure I made this clear enough. So I will try to
do it now.

According to my knowledge these conditional compilation
constructs based on have DIRECT_H came in about 2002/2003.
Long time has passed since and I am not familiar with all
compilation environments of libdvdcss on Windows.

There seems something wrong with this code nowadays but
maybe it was even back then. I can't know for sure.

What I know is that:

* On OS/2 you do not need to include direct.h and you
should never take the code path under "#ifdef HAVE_DIRECT_H"
later in the code, because it fails to compile and
OS/2 code is actually in the #else part

* On Cygwin there is no direct.h for me

* On MinGW there is direct.h but it is seems not to be needed
in my version. At least everything compiles fine. It may have
been needed at some point.

* Regardign MSVC I do not know. It doesn't seem impossible
that it needs/needed direct.h but I did not yet find docs
that state a non-underscored version is/was available.

So I tested removing direct.h #inlcude completely and
putting the remaining "#ifdef HAVE_DIRECT_H" block under
"#ifdef WIN32". This is identical to the patch Diego sent
and it works for me on Linux, Cygwin and MinGW. It should
surely work on OS/2 too.
Post by Jean-Baptiste Kempf
This patch is even more wrong than the previous ones: where are
direct.h functions being used on this chunk of code?
This is the most conservative solution I could think of.
It does not behave any different than before besides it
only does so on platforms where WIN32 is defined. So if
HAVE_DIRECT_H is defined on e.g. OS/2 it still will behaved
as if it was not defined. It therefore un-breaks OS/2 build
for MPlayer.

I thought it might be of value in case we don't find testers
for all platforms involved.

If we can safely drop HAVE_DIRECT_H that might be of course
the best solution.

Alexander

Loading...