[MPlayer-cvslog] r34203 - in trunk: Changelog DOCS/tech/crosscompile.txt DOCS/tech/mingw-crossc

[Ingo Brückl]

Reimar Döffinger wrote on Sat, 15 Oct 2011 21:45:41 +0200:

>> Author: ib
>> Date: Sat Oct 15 15:20:52 2011
>> New Revision: 34203
>>
>> Log:
>> Port to Wine.

> I am not really happy about having all these wildly different things in
> one single commit, particularly since I do not see any reason for them
> to be.

Shall I revert? If so, which changes would you like to be in separate
commits?

Having all changes necessary for the Wine port in one single commit made
sense to me, but I see that Makefile and loader/com.h are not closely related
to that.

> The osdep/priority.c seems like a code correctness change that can be
> motivated without any need for wine to come in.

How to explain the defined(__WINE__) then?

>> +  ./configure --target=$(uname -m)-wine --cc=winegcc --windres=wrc

> That doesn't work on 64 bit Linux. I have not finished compiling yet,
> but I think the only sensible command to suggest is
> ./configure --target=i686-wine --cc="winegcc -m32" --windres=wrc

I'm not familiar with 64 bit Linux or non-Intel CPUs, but isn't there a
64-bit Wine as well and doesn't Wine run even on some non-Intel CPUs?

What if my CPU isn't an i686, but an i[345]86?

If really necessary, the "-m32" should be in the extra_cflags in the
configure script.

>> +#ifdef __WINE__
>> +/**
>> + * @brief Convert a Windows style file name into an Unix style one.
>> + *
>> + * @param filename pointer to the file name to be converted
>> + *
>> + * @return pointer to the converted file name

> You should say what the point of that code is.

That would be the first time to document the point of a function in addition
to describing what it does, but I've added a comment where it is called.

Is this the right place to explain Wine's drive mapping or why a Windows
program can have Windows style file name arguments?

>> Modified: trunk/loader/com.h
>>
>> +// hack: use copies of the IIDs
>> +#define IID_IUnknown MP_IID_IUnknown
>> +#define IID_IClassFactory MP_IID_IClassFactory
>>  extern const GUID IID_IUnknown;
>>  extern const GUID IID_IClassFactory;

> As said, I disagree that this is a hack.

Comment revised.

>> Modified: trunk/mplayer.c
>> +#ifndef __WINE__
>>      signal(SIGSEGV, exit_sighandler); // segfault
>> +#endif

> This however is a huge ugly hack and should have a comment at least.

Comment added.

Reimar Döffinger wrote on Sat, 15 Oct 2011 21:49:10 +0200:

>> Modified: trunk/Makefile
>>
>>  %-rc.o: %.rc
>> -     $(WINDRES) -I. $< $@
>> +     $(WINDRES) -I. $< -o $@

> This does not work for me anyway:

> This works though:
> wrc -I/usr/include/wine/windows -I. osdep/mplayer.rc -o
> osdep/mplayer-rc.o

> I'd conclude wrc is broken in debian-unstable libwine-dev-unstable
> version...

If resource file (which I'm unfamiliar with) do include header files, I'd
agree. wrc should have the same built-in include directories as winegcc then.

Ingo