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

[Steve Lhomme]

M?ns Rullg?rd wrote:
> Michael Niedermayer said:
>> 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
> 
> With some proper care, a portable way of disabling tprintf is
> 
> #define tprintf if(0)

Yes, this works with MSVC too without the __STDC_VERSION__

>>> +#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
> 
> This is a somewhat dangerous thing to do, since it makes sizeof(name)
> unpredictable.

I just checked, all the variables with this kind of change are never 
used with sizeof().

>>> 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 ...
> 
> Yes, on 64-bit targets.  Casting to intptr_t should be ok, as should long.
> (The Linux kernel depends on long being compatible with pointers.)

Yes, intptr_t works within MSVC too.