[FFmpeg-devel] Various small fixes

Stefano Sabatini stefano.sabatini-lala
Sat Dec 13 13:57:52 CET 2008


On date Saturday 2008-12-13 13:34:27 +0100, Anders Gr?nberg encoded:
> I have been experimenting with FFmpeg and this patch fixes some small
> issues I found in the code base that should be beneficial to other
> FFmpeg developers.

Please put each different functional change in a different patch. This
way review and application will be easier.

> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 16089)
> +++ ffmpeg.c	(working copy)
> @@ -87,7 +87,7 @@
>      int in_file;
>  } AVMetaDataMap;
>  
> -static const OptionDef options[];
> +const OptionDef options[];
>  
>  #define MAX_FILES 20
>  
> Index: ffserver.c
> ===================================================================
> --- ffserver.c	(revision 16089)
> +++ ffserver.c	(working copy)
> @@ -60,7 +60,7 @@
>  const char program_name[] = "FFserver";
>  const int program_birth_year = 2000;
>  
> -static const OptionDef options[];
> +const OptionDef options[];

Why these? They break compilation here, the forward declaration have
to match with the actual declaration, also the options are only used
in those files so they should be static.

[...]

> Index: libavutil/common.h
> ===================================================================
> --- libavutil/common.h	(revision 16089)
> +++ libavutil/common.h	(working copy)
> @@ -108,7 +108,7 @@
>  /* assume b>0 */
>  #define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b))
>  #define FFABS(a) ((a) >= 0 ? (a) : (-(a)))
> -#define FFSIGN(a) ((a) > 0 ? 1 : -1)
> +#define FFSIGN(a) ((a) >= 0 ? 1 : -1)

I think this may break something. Anyway the name FFSIGN looks poorly
chosen to me, *either* semantically wrong.

Maybe it should be:
#define FFSIGN(a) ((a) > 0 ? 1 : (a) < 0 ? -1 : 0)

Regards.
-- 
FFmpeg = Freak and Fiendish Mortal Powered Explosive Gem




More information about the ffmpeg-devel mailing list