[Ffmpeg-devel] [PATCH] from DivX, Part 7: MSVC fixes

Michael Niedermayer michaelni
Wed Jan 25 13:12:45 CET 2006


Hi

On Fri, Dec 16, 2005 at 01:49:16PM -1000, Steve Lhomme wrote:
> These are only the "soft" part of the MSVC fixes. That means I didn't 
> include parts I know wouldn't make it, like the named fields in structures.

[...]
>  #else //TRACE
> -#define tprintf(...) {}
> + #if __STDC_VERSION__ >= 199901L
> +  #define tprintf(...) {}
> + #else
> +  inline void tprintf(char *x, ...) {}
> + #endif  
>  #endif

hmm, are you sure that 199901L is the oldest which supports this? and even
if so, isnt it better to check for the compiler instead of the standard?
a compiler might support 98% of C99 but might not set 
__STDC_VERSION__ == 199901L as it doesnt support 100%
this comment applies to all __STDC_VERSION__ changes


[...]
> +#if (__STDC_VERSION__ >= 199901L) && defined(CONFIG_ENCODERS)
>  static inline int w_c(void *v, uint8_t * pix1, uint8_t * pix2, int line_size, int w, int h, int type){
>  #ifdef CONFIG_SNOW_ENCODER //idwt is in snow.c
>      int s, i, j;
> @@ -389,6 +389,7 @@
>  static int w97_16_c(void *v, uint8_t * pix1, uint8_t * pix2, int line_size, int h){
>      return w_c(v, pix1, pix2, line_size, 16, h, 0);
>  }
> +#endif

and what is the problem with that code?


[...]
> +#if __STDC_VERSION__ >= 199901L
>      uint8_t fixed[s->mb_stride * s->mb_height];
> +#else
> +    uint8_t *fixed=(uint8_t*)alloca(s->mb_stride * s->mb_height);
> +#endif    

rejected

#define A(type, name, size) type name[size];
#define A(type, name, size) type *name= alloca(size * sizeof(type));

would be cleaner but even then i would say that needs some disscussion and
should be a seperate patch


[...]
> Index: libavcodec/h263.c
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/h263.c,v
> retrieving revision 1.292
> diff -u -r1.292 h263.c
> --- libavcodec/h263.c	12 Dec 2005 01:56:45 -0000	1.292
> +++ libavcodec/h263.c	16 Dec 2005 23:10:06 -0000
> @@ -39,6 +39,9 @@
>  #include "h263data.h"
>  #include "mpeg4data.h"
>  
> +#ifdef CONFIG_WIN32
> +#include <windows.h>
> +#endif

rejected, this doesn belong in here


[...]
> Index: libavcodec/loco.c
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/loco.c,v
> retrieving revision 1.5
> diff -u -r1.5 loco.c
> --- libavcodec/loco.c	8 Apr 2005 21:34:48 -0000	1.5
> +++ libavcodec/loco.c	16 Dec 2005 23:13:42 -0000
> @@ -241,14 +241,14 @@
>          l->lossy = 0;
>          break;
>      case 2:
> -        l->lossy = LE_32(avctx->extradata + 8);
> +        l->lossy = LE_32((int8_t*)avctx->extradata + 8);

hmmmm, IMHO make the extradata field in AVCodecContext uint8_t * instead


[...]
>  
> Index: libavcodec/mjpeg.c
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/mjpeg.c,v
> retrieving revision 1.114
> diff -u -r1.114 mjpeg.c
> --- libavcodec/mjpeg.c	18 Sep 2005 21:21:01 -0000	1.114
> +++ libavcodec/mjpeg.c	16 Dec 2005 23:16:01 -0000
> @@ -495,7 +495,7 @@
>      int size= put_bits_count(&s->pb) - start*8;
>      int i, ff_count;
>      uint8_t *buf= s->pb.buf + start;
> -    int align= (-(size_t)(buf))&3;
> +    int align= (-(int)(buf))&3;

this is going to trigger warnings on some compilers i think ...


[...]
>      ScanTable intra_v_scantable;
>      ScanTable inter_scantable; ///< if inter == intra then intra should be used to reduce tha cache usage
> Index: libavcodec/msmpeg4.c
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/msmpeg4.c,v
> retrieving revision 1.90
> diff -u -r1.90 msmpeg4.c
> --- libavcodec/msmpeg4.c	26 Aug 2005 19:05:44 -0000	1.90
> +++ libavcodec/msmpeg4.c	16 Dec 2005 23:18:34 -0000
> @@ -29,6 +29,10 @@
>  #include "dsputil.h"
>  #include "mpegvideo.h"
>  
> +#ifdef CONFIG_WIN32
> +#include <windows.h>
> +#endif

sorry, no we wont include OS specific stuff in random files


[...]
> +    if (avctx->sub_id >= 0x20100000 && avctx->sub_id <= 0x2019ffff)
> +        s->low_delay=1;
> +    else if (avctx->sub_id >= 0x20200002 && avctx->sub_id <= 0x202fffff)
> +    {
> +        s->low_delay=0;
> +        s->avctx->has_b_frames=1;
> +    }
> +    else
>      switch(avctx->sub_id){
>      case 0x10000000:
>          s->rv10_version= 0;
> @@ -541,13 +549,11 @@
>      /*case 0x20100001:
>      case 0x20101001:
>      case 0x20103001:*/
> -    case 0x20100000 ... 0x2019ffff:
>          s->low_delay=1;
>          break;
>      /*case 0x20200002:
>      case 0x20201002:
>      case 0x20203002:*/
> -    case 0x20200002 ... 0x202fffff:

rejected, dont replace clean code with such a mess


[...]
> -typedef struct {
> -} WSSNDContext;
> -
>  static const char ws_adpcm_2bit[] = { -2, -1, 0, 1};
>  static const char ws_adpcm_4bit[] = {
>      -9, -8, -6, -5, -4, -3, -2, -1,
> @@ -137,7 +134,7 @@
>      "ws_snd1",
>      CODEC_TYPE_AUDIO,
>      CODEC_ID_WESTWOOD_SND1,
> -    sizeof(WSSNDContext),
> +    0,

now while this is correct it doesnt belong in this patch, and must be split
out


> Index: libavformat/barpainet.c
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/barpainet.c,v
> retrieving revision 1.1
> diff -u -r1.1 barpainet.c
> --- libavformat/barpainet.c	2 Nov 2002 10:35:07 -0000	1.1
> +++ libavformat/barpainet.c	31 Aug 2005 10:02:17 -0000
> @@ -1,6 +1,9 @@
>  
> +#include "avformat.h"
>  #include <stdlib.h>
> +#ifndef CONFIG_WIN32
>  #include <strings.h>
> +#endif
>  #include "barpainet.h"

wtf? isnt that beos specific? what does a CONFIG_WIN32 do in a beos specific
file?


[...]
> Index: libavformat/nsvdec.c
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/nsvdec.c,v
> retrieving revision 1.11
> diff -u -r1.11 nsvdec.c
> --- libavformat/nsvdec.c	12 Dec 2005 01:56:46 -0000	1.11
> +++ libavformat/nsvdec.c	16 Dec 2005 23:32:26 -0000
> @@ -362,7 +362,7 @@
>          if((unsigned)table_entries >= UINT_MAX / sizeof(uint32_t))
>              return -1;
>          nsv->nsvf_index_data = av_malloc(table_entries * sizeof(uint32_t));
> -#warning "FIXME: Byteswap buffer as needed"
> +/* #warning "FIXME: Byteswap buffer as needed" */

sorry no, there is #ifdef for that


[...]

-- 
Michael





More information about the ffmpeg-devel mailing list