[FFmpeg-devel] [PATCH] Alternative OS/2 patch
Tue Dec 4 07:38:02 CET 2007
On 11/27/07 02:34 am, Diego Biurrun wrote:
> On Mon, Nov 26, 2007 at 06:11:54PM -0800, Dave Yeo wrote:
>> On 11/26/07 04:53 pm, Diego Biurrun wrote:
>>> On Fri, Nov 23, 2007 at 09:58:42PM -0800, Dave Yeo wrote:
>>>> On 11/08/07 09:16 pm, Dave Yeo wrote:
>>>>> Attached patch cleans up some of the nasty escaping by putting the
>>>>> DESCRIPTION field into a separate variable. This has the extra advantage
>>>>> that it makes it simpler to change the description. (adding things like
>>>>> vendor or bldlevel info).
>>>>> Also makes config.mak more readable :)
>>> The DESCRIPTION variable is only used once. How does that make things
>> The DESCRIPTION needs to be quoted (according to the docs with single
>> quotes though double quotes do work) so we need more escaping for the
>> quotes to be written to the DEF file. Separating it allows only the
>> DESCRIPTION variable to contain escapes. It also makes it simpler to expand
>> the DESCRIPTION to be expanded by anyone who is distributing FFmpeg.
> I am against moving stuff into variables that is only used once.
Well simplest is just to remove it. Patch looks cleaner and it is easy
enough for someone to add it back if they want to use the DESCRIPTION field.
Better to have the versioning in the file name anyways, see attached patch.
>> --- configure (revision 11093)
>> +++ configure (working copy)
>> @@ -1217,6 +1219,35 @@
>> + os/2*)
>> + DESCRIPTION="'\"\$(SLIBNAME_WITH_VERSION)\"'"
>> + SHFLAGS='$(FULLNAME).def -Zdll -Zomf'
>> + SLIB_CREATE_DEF_CMD='echo LIBRARY $(FULLNAME) INITINSTANCE TERMINSTANCE > $(FULLNAME).def; \
>> + echo DESCRIPTION $(DESCRIPTION) >> $(FULLNAME).def; \
>> + echo PROTMODE >> $(FULLNAME).def; \
>> + echo CODE PRELOAD MOVEABLE DISCARDABLE >> $(FULLNAME).def; \
>> + echo DATA PRELOAD MOVEABLE MULTIPLE NONSHARED >> $(FULLNAME).def; \
>> + echo EXPORTS >> $(FULLNAME).def; \
>> + emxexp -o $(OBJS) >> $(FULLNAME).def'
>> + SLIB_EXTRA_CMD='emximp -o $(LIBPREF)$(FULLNAME)_dll.a $(FULLNAME).def; emximp -o $(LIBPREF)$(FULLNAME)_dll.lib $(FULLNAME).def'
>> + SLIB_INSTALL_EXTRA_CMD='install -m 644 $(LIBPREF)$(FULLNAME)_dll.lib $(LIBPREF)$(FULLNAME)_dll.a "$(LIBDIR)"'
>> + SLIB_UNINSTALL_EXTRA_CMD='rm -f "$(LIBDIR)"/$(LIBPREF)$(FULLNAME)_dll.lib;rm -f "$(LIBDIR)"/$(LIBPREF)$(FULLNAME)_dll.a'
> nit: Please keep the same order of _dll.lib and _dll.a in the last three
Fixed in attached patch.
> I am starting to think that we should introduce a variable for DEF
> files, MinGW could use it as well.
Generally in OS/2 only makefiles (and some Windows makefiles) you'd have
a rule something like DEFFILE to create the def file. Not really sure
how a variable would work as currently we need to run commands to
extract the exports from the objects. I'd imagine there are also similar
tools available for Windows but since GCC seems to handle it fine right
The main reason to use a DEF file on platforms that don't need it would
be to limit the exports to public ones. Some projects come with DEF
files defined for this reason, eg see zlib.
Anyways the talk of libossupport reminded me of how the patch was
broken, it depended on short library names. Stupid 8.3 limitation for
the kernel DLL loader, something OS/2 developers routinely bitch about
especially since ver 2.0 handled longname DLLs fine.
Anyways tweeked the patch to handle longer names and at the same time
added $LIBMAJOR to the DLL that we link against. Should be good until
the major version number gets to 100 :) Only small drawback is it non
obvious which DLL gets linked against.
The only other conflict is right now I'm exporting by ordinal number so
adding an export (unless at the end) will break the dll. Exporting by
name would allow a newer DLL to replace an older one and keep on working
as long as the API is stable during a major version.
> Otherwise the patch looks OK.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 3290 bytes
Desc: not available
More information about the ffmpeg-devel