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

[Steve Lhomme]

Michael Niedermayer wrote:
> 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

Nop, I don't know. But I think it's a safe choice.

> if so, isnt it better to check for the compiler instead of the standard?

I usually try to avoid checking for compilers when it's a general case. 
Because there are so many compilers around, when you use a new one you 
can't trust to code to be tuned or not for it.

> 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

I changed all the __STDC_VERSION__ to test __STDC_VERSION__ <= 199901L. 
In the case __STDC_VERSION__ is not defined (in MinGW you need to use 
-std=gnu99) the default case applies.

>> +#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?

For that one, I don't remember. But snow.c is now disabled in MSVC 
builds as there are too many errors.

>> +#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

Given __align8(type, var) was rejected because it's "unreadable", I 
don't think this macro would qualify neither. Other than that, I agree 
this is a better long-term change.

>> 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

You mean it should be made more general ?

>> 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

If that can be accepted, I'd be happy with that change.

>> 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?

Given we don't compile that one neither, forget about it.

>> 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

#ifdef warning ? How do you know if a compiler has a specific macro ?

Steve