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

[Michael Niedermayer]

Hi

On Fri, Jan 27, 2006 at 09:11:42AM -1000, Steve Lhomme wrote:
> Hi
> 
> Michael Niedermayer wrote:
> >Hi
> >
> >On Thu, Jan 26, 2006 at 12:16:21PM -1000, Steve Lhomme wrote:
> >>Michael Niedermayer wrote:
> >>>Hi
> >>>
> >>>On Thu, Jan 26, 2006 at 01:04:44AM -0500, Rich Felker wrote:
> >>>>On Thu, Jan 26, 2006 at 12:39:46AM +0100, Michael Niedermayer wrote:
> >>>>>Hi
> >>>>>
> >>>>>On Wed, Jan 25, 2006 at 01:30:44PM -1000, Steve Lhomme wrote:
> >>>>>>Michael Niedermayer wrote:
> >>>>>>>>>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.
> >>>>>>>who rejected __align8(type, var)? i didnt or at least i dont 
> >>>>>>>remember it
> >>>>>>I think Felker was the one against it.
> >>>>I don't have the authority to reject, just the authority to flame. :)
> >>>:)
> >>>
> >>>
> >>>>>__align8(type, var) is ok, ill apply it if you send a patch with it
> >>>>I was against it because it's not even obvious that this is a variable
> >>>>declaration without reading the #define.
> >>>>
> >>>>Also, using __ prefix is illegal. __ is reserved for the
> >>>>implementation. For all you know, the C library/compiler might define
> >>>>__align8 for its own purposes. You should use align8 or something not
> >>>>in the reserved namespace at the very least.
> >>>yes fully agree, i missed the __ in there, dunno must have been tired or
> >>>something
> >>>
> >>>maybe we should borrow the
> >>>#define DECLARE_ALIGNED( type, var, n ) type var 
> >>>__attribute__((aligned(n)))
> >>>from x264?
> >>
> >>
> >>Here is the patch, I used DECLARE_ALIGNED_8() and DECLARE_ALIGNED_16()
> >>I might be better than having a "random" alignement size, especially 
> >>since some sizes are mapped to different values that the one set on some 
> >>platforms...
> >
> >[...]
> >>-uint16_t __align8 inv_zigzag_direct16[64] = {0, };
> >>+static DECLARE_ALIGNED_8(uint16_t,inv_zigzag_direct16_static[64]) = {0, 
> >>};
> >
> >that doesnt look good
> 
> And it didn't build in MinGW. Here is the fixed patch.

patch looks ok, can be applied if it compiles & regression tests pass

[...]

-- 
Michael