[FFmpeg-devel] [PATCH] DECLARE_ALIGNED

Pavel Pavlov pavel
Thu Jun 4 21:03:06 CEST 2009


> -----Original Message-----
> We prefer patches made with svn diff from the root directory, 
> so they can be applied with patch -p0 < blah. I for example 
> keep one clean svn tree just for the purpose of creating 
> diffs if they're part of some big changes, and it really 
> helps us out in applying & reviewing patches.
> 
> > Let me know if what I have sent is unusable so I wouldn't 
> waste my and 
> > others' time...
> 
> Your patches are welcome, but they'll be more welcome after a 
> few minor tweaks (it's just standard procedure around here).


Well, I have tortoise svn, and I also have two c:\FFMPEG and C:\-FFMPEG,
the second one is 
unmodified version for creating the diffs and comparing my changes.
On windows I right click and from svn submenu there is "Create patch
option", that's what 
I did.
Maybe you can manually modify diffs I sent so that they could be
applied?

From

Index: C:/FFMPEG/libswscale/swscale_internal.h
===================================================================
--- C:/FFMPEG/libswscale/swscale_internal.h	(revision 29346)
+++ C:/FFMPEG/libswscale/swscale_internal.h	(working copy)

To
Index: libswscale/swscale_internal.h
===================================================================
--- libswscale/swscale_internal.h	(revision 29346)
+++ libswscale/swscale_internal.h	(working copy)

Is that's all required?



> fdct_mmx.c: Who maintains it? What is up with the "one single 
> array within a struct" mess, can we get rid of the struct 
> somehow, otherwise this is quite unreadable IMO.
> idct_mmx.c: the \s at the end of the lines should stay aligned.

I also noticed that mess; what's the point to declare aligned struct,
and 
then member of the struct?.. Maybe on some comilers generated code
wasn't 
correct and that fixed the problem. If it is not a fix and isn't
required 
then definetly that mess should be removed :)



> Well... That it doesn't change anything in that regard 
> doesn't mean that it can't be wrong before and after.
> Also I only meant to explain the specific purpose of 
> DECLARE_ASM_CONST, which maybe isn't be best-chose name.
> 
> > Check all the diffs I sent. They are small and you can 
> inspect all of 
> > them in 5 minutes.
> 
> Yes, they seem ok in that regard, those files don't really 
> use MANGLE much.

I understand what's the meaning and reason for DECLARE_ASM_CONST
And as I understand DECLARE_ASM_CONST should be used only for 
variables that are referenced as part of MANGLE'd inline asm code.
I can't confirm about gcc, but icl definetly has that problem with 
standalone 64 bit static const variables - it doesn't try to use
it by referencing it, but it tries to use it as immidiate value 
(copies it to stack and then referneces it through stack pointer).
I don't know if it whould actually be better in 64 bit code, but in
32-bit it's just a big preformance hit. And in most of the places it 
wouldn't compile inline asm, because it required extra registers to
do the copying. I fixed it for inline asm, but all the places that 
have 64 bit consts in c code will have that problem.



More information about the ffmpeg-devel mailing list