[FFmpeg-devel] [PATCH] remove MSVC cruft

[Michael Niedermayer]

On Wed, Feb 13, 2008 at 09:37:04AM +0100, Diego Biurrun wrote:
> On Tue, Feb 12, 2008 at 04:15:10PM +0100, Michael Niedermayer wrote:
> > On Tue, Feb 12, 2008 at 09:14:10AM +0100, Diego Biurrun wrote:
> > > On Tue, Feb 12, 2008 at 01:53:59AM +0100, Michael Niedermayer wrote:
> > > > On Tue, Feb 12, 2008 at 12:55:15AM +0100, Diego Biurrun wrote:
> > > > > On Sat, Feb 09, 2008 at 08:10:05PM +0100, Diego Biurrun wrote:
> > > > > > On Sat, Feb 09, 2008 at 02:51:53PM +0100, Michael Niedermayer wrote:
> > > > > > > On Sat, Feb 09, 2008 at 02:09:04PM +0100, Diego Biurrun wrote:
> > > > > > > > On Sat, Feb 09, 2008 at 02:06:01PM +0100, Diego Biurrun wrote:
> > > > > > > > > As noted by Reimar, the following two lines in libavutil/mem.h are
> > > > > > > > > probably MSVC cruft:
> > > > > > > > > 
> > > > > > > > >   #define DECLARE_ALIGNED(n,t,v)      __declspec(align(n)) t v
> > > > > > > > >   #define DECLARE_ASM_CONST(n,t,v)    __declspec(align(n)) static const
> > > > > > > > > 
> > > > > > > > > I suggest the attached patch removing them.
> > > > > > > > 
> > > > > > > > *sigh*
> > > > > > > 
> > > > > > > I am against it.
> > > > > > > 
> > > > > > > it rather should be
> > > > > > > #elif HAVE_DECLSPEC
> > > > > > > #define DECLARE_ALIGNED(n,t,v)      __declspec(align(n)) t v
> > > > > > > #define DECLARE_ASM_CONST(n,t,v)    __declspec(align(n)) static const t v
> > > > > > > #else
> > > > > > > #warning no align and asm directives, this might fail
> > > > > > > #define DECLARE_ALIGNED(n,t,v)      t v
> > > > > > > #define DECLARE_ASM_CONST(n,t,v)    static const t v
> > > > > > > #endif
> > > > > > > 
> > > > > > > There are people who maintain a hacked up version of ffmpeg which does
> > > > > > > compile under msvc. Theres no need to make their work harder by removing
> > > > > > > clean and seperated code. Its only the messy parts which we should reject.
> > > > > > 
> > > > > > These two lines do not help them.  Also, this code is not clean and
> > > > > > separated, it is the fallback when __GNUC__ is not defined.  So anybody
> > > > > > trying compiler X will run into problems.  I don't see anybody coming
> > > > > > up with a proper configure check.  Until then these lines do more harm
> > > > > > than good and should IMO be removed.
> > > > > 
> > > > > May I insist here?  The fallback case should definitely not be MSVC,
> > > > > i.e. an unsupported and non-standards-compliant compiler.
> > > > 
> > > > I dont see what you complain about.
> > > 
> > > I complain about having MSVC be the fallback case, a compiler we
> > > actively decided to discontinue supporting.
> > 
> > We agree here ...
> > 
> > > > #ifdef GNUBLAH
> > > > ...
> > > > #elif HAVE_DECLSPEC
> > > > #define DECLARE_ALIGNED(n,t,v)      __declspec(align(n)) t v
> > > > #define DECLARE_ASM_CONST(n,t,v)    __declspec(align(n)) static const t v
> > > > #else
> > > > #warning no align and asm directives, this might fail
> > > > #define DECLARE_ALIGNED(n,t,v)      t v
> > > > #define DECLARE_ASM_CONST(n,t,v)    static const t v
> > > > #endif
> > > > 
> > > > As i suggested would change the default/fallback to normal C. There is nothing
> > > > in the code that would require a change to configure. Nothing will blow up
> > > > if HAVE_DECLSPEC is not defined.
> > > 
> > > Nothing will blow up if we remove the __declspec lines either.
> > > Pretending that we support MSVC syntax is misleading. 
> > 
> > > I am against
> > > using HAVE_ definitions in the code which configure does not set.
> > 
> > Iam against creating artifical dependancies. If one person sends me a clean
> > and well written implementation of something we want, that should be applied.
> > An example might be a implementation of all jpeg 2000 wavelets. Or the
> > AVFilter core. There is no sense in making that dependant on all other parts.
> > 
> > So my decission stands, the __declspec stays, it should not be default though.
> 
> Implemented.

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080213/f679614a/attachment.pgp>