"Ronald S. Bultje" <rsbul...@gmail.com> writes:

> Hi,
>
> see attached. It's kinda big, some parts can/should be done
> differently, but it's a starting point so let's start talking about
> how to get this in the right way.

If you split the patch into logically independent "fixes", reviewing it
might be almost possible.  As is, I refuse to even begin looking at it.

> Some general comments:
> - only tested on 32bit so far
> - no it won't compile on non-MSVC with my patch (I had to break the
> Makefile in a couple of places to get linking to work)

That's fine for a work in progress.  We can fix it properly later.

> - the include of stdlib.h everywhere is because of [1]. We may need to
> add av_restrict instead.

I fail to see how that ph0rum post suggests including stdlib.h
everywhere.  The problem is that msvc doesn't support the restrict
keyword.  You'll have remove it with a custom preprocessor or something,
just like you have to remove designated initialisers.

> - the include of mathematics.h, avconfig.h and avstring.h everywhere
> is (I think) legit, to account for snprintf(), M_PI or inline.

I seriously doubt that those are really needed in all the places you've
added them.

> - the added assembly in fmtconvert.asm helps to get rid of a VLA in
> fmtconvert_mmx.c.

Please submit that as a separate patch.  It's a good thing on its own.

> - test.c converts c99 to c89 which MSVC can compile. Don't look at it
> unless you want to see true pain, it's intended to be shipped outside,
> like gas-preprocessor.pl. (unit.c and unit2.c are unittests for it.)

Then please do not include those in the patch.

> - the renames of libavutil/x86/cpu.c, libavcodec/x86/mlpdsp.c and
> libswscale/x86/rgb2rgb.c are to prevent duplicate filenames in the
> same library, which makes the MSVC linker crap out in debug mode
> (works fine in release mode).

Seriously?

-- 
Måns Rullgård
m...@mansr.com
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to