[FFmpeg-devel] [PATCH] Add versioning information to dlls

[Jeremy Kolb]

Diego Biurrun wrote:
> On Thu, May 22, 2008 at 03:27:19PM -0400, Jeremy Kolb wrote:
>   
>> This should take care of the trailing whitespaces.
>>     
>
> Ignoring for a moment that this patch has other more serious problems
> because of which it was reverted...
>
>   
>> --- configure	(revision 13216)
>> +++ configure	(working copy)
>> @@ -1236,12 +1237,15 @@
>>          SLIB_EXTRA_CMD='-lib /machine:$(LIBTARGET) /def:$$(@:$(SLIBSUF)=.def) /out:$(SUBDIR)$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.lib)'
>> +        SLIB_EXTRA_OBJS="dllinfo.rco"
>> +        SLIB_EXTRA_DEP='$(SUBDIR)../version.h'
>>     
>
> nit: Reverse the order to sort this alphabetically.
>
>   
>> @@ -2069,6 +2073,8 @@
>>      echo "SLIB_EXTRA_CMD=${SLIB_EXTRA_CMD}" >> config.mak
>> +    echo "SLIB_EXTRA_OBJS=${SLIB_EXTRA_OBJS}" >> config.mak
>> +    echo "SLIB_EXTRA_DEP=${SLIB_EXTRA_DEP}" >> config.mak
>>     
>
> ditto
>
>   
>> @@ -2215,3 +2221,69 @@
>> +
>> +# build dll configuration for windows.
>>     
>
> Build DLL configuration for Windows.
>
>   
>> +dllinfo_generate() {
>> +    name=$1
>> +    description=$2
>> +    version=$3
>> +    major=${version%%.*}
>> +    version_commas=`echo $version | tr . ,`,0
>> +    dllname=`echo $name | sed s/lib//`
>>     
>
> dllname=${name#lib}
>
>   
>> +
>> +    license_str="This FFmpeg build is distributed under the terms of the GNU $license.\r\n"
>> +    test "$license" = "unredistributable" &&
>> +        license_str="This FFmpeg build may not be distributed publicly.\r\n"
>>     
>
> This FFmpeg build contains non-free code and is not legally redistributable.
>
>   
>> +    cat <<EOF >$TMPRC
>>     
>
> Space after the > please.
>
>   
>> +#include <WinVer.h>
>> +#include "version.h"
>> +VS_VERSION_INFO VERSIONINFO
>> + FILEVERSION $version_commas
>> + PRODUCTVERSION $version_commas
>> + FILEFLAGSMASK 0x17L
>> + FILEFLAGS 0x0L
>> + FILEOS VOS__WINDOWS32
>> + FILETYPE VFT_DLL
>> + FILESUBTYPE 0x0L
>>     
>
> Is there a reason for this weird 1-space indentation?
>
>   

Seems to be convention but I can remove it.

>> +BEGIN
>> +    BLOCK "StringFileInfo"
>> +    BEGIN
>> +        BLOCK "040904b0"
>> +        BEGIN
>> +            VALUE "Comments",            "$license_str"
>> +                                         "Source code is available at http://ffmpeg.org"
>>     
>
> Add a trailing / to the URL and a period to the sentence.
>
>   
>> +    if ! cmp -s $TMPRC $name/dllinfo.rc; then
>> +        mv -f $TMPRC $name/dllinfo.rc
>> +    fi
>> +    rm -f $TMPRC
>>     
>
> Is this necessary?  We regenerate config.mak and the pkg-config files
> unconditionally.  config.h is treated this way because unnecessarily
> recreating it triggers a lot of pointless recompiles.
>
>   
I guess not, I can just output to those files directly.  I did that 
because an earlier patch did it.
>> +if test "$dllinfo" = "yes"; then
>> +    dllinfo_generate libavutil "FFmpeg utility library" "$LIBAVUTIL_VERSION"
>> +    dllinfo_generate libavcodec "FFmpeg codec library" "$LIBAVCODEC_VERSION"
>> +    dllinfo_generate libavformat "FFmpeg container format library" "$LIBAVFORMAT_VERSION"
>> +    dllinfo_generate libavdevice "FFmpeg device handling library" "$LIBAVDEVICE_VERSION"
>> +    dllinfo_generate libavfilter "FFmpeg video filtering library" "$LIBAVFILTER_VERSION"
>> +    dllinfo_generate libpostproc "FFmpeg post processing library" "$LIBPOSTPROC_VERSION"
>>     
>
> postprocessing
>
> Also, you could align all those lines.
>
>   
>> --- subdir.mak	(revision 13216)
>> +++ subdir.mak	(working copy)
>> @@ -27,7 +27,9 @@
>>  
>> +$(SUBDIR)$(SLIB_EXTRA_OBJS): $(SLIB_EXTRA_DEP)
>>     
>
> I think you could just generate a proper .d file from the .rco file and
> be done with it.  This would simplify things considerably.
>
>   

How do you mean?  What are .d files?  How do I do that?

>> -$(SUBDIR)$(SLIBNAME_WITH_MAJOR): $(OBJS)
>> +$(SUBDIR)$(SLIBNAME_WITH_MAJOR): $(OBJS) $(SUBDIR)$(SLIB_EXTRA_OBJS)
>>     
>
> Alternatively, add that extra object to OBJS.  This should make things
> simpler.  It also likely fixes the build failures we were seeing on some
> platforms.
>
>   

Ok, which one should I do?  The .d file or add the extra object?

> Diego
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
>   

Jeremy