alignment issues for deinterlacers
Roland Scheidegger <[email protected]>
| Newsgroups | gmane.comp.video.xine.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi, ok software deinterlacers may be a bit out of fashion these days (with hw decoding...) but nevertheless I was playing with that a bit as greedy2frame was too slow for me (with h.264 full hd streams). So first thing I did was convert it to use sse2 instead of mmx. Not quite unexpectedly it didn't actually help performance (for two reasons, one is the algorithm is nearly completely memory bandwidth bound, and second even if it wouldn't be memory bandwidth bound the cpu in question is a K8 which has only 64bit wide simd units anyway). Still, I think it's about time to use sse2. However, a big problem I encountered was there seems to be absolutely no guarantee about ANY alignment of either source or destination images. This is ok for mmx which doesn't require any alignment (though performance might suck if things aren't quite 64bit aligned) but is a problem for sse which requires 128bit aligned addresses. Loading the images from memory could be made to work with movdqu instead of movdqa (which wasn't much of a loss in my measurements) but storing uses streaming stores for which there is no direct unaligned replacement (and not using streaming stores is NOT an option as the performance hit is way too big). So I'm wondering if there is some possibility to guarantee alignment? If not I guess the code just needs to deal with this (for instance do beginning of line specially before hitting aligned part), which seems to increase code complexity tremendously (i.e. the code just having to deal with alignment issues could be easily as complex as the rest of the code). In any case so you get an idea what I'm talking about I'm attaching a greedy2frame version for sse2 (it is only enabled on x86_64 though there's not really a reason why it wouldn't work on x86 too as I didn't use the upper 8 xmm regs). It is ~25% faster or so than the old version (which is nearly enough here to make it go from useless to near perfect), though the actual speedup is not related to sse2 really - incorporating the line copy into the loop helped (I _think_ this is because the xine_fast_memcpy using libc memcpy caused some of the data reused later to be loaded twice from memory by using prefetchnta due to bank conflicts in the L1), and the prefetch instructions mostly did the rest (not sure why actually, k8 should have decent hw prefetcher and the pattern is regular enough, but it still helped). Note that I actually never saw a crash due to misalignment - but I think that's mostly just "luck", not even malloc guarantees you the required 16byte alignment. I'm quite sure some odd-sized video would cause a GPF. (btw I'm thinking about changing the deinterlace interface to handle both frames for the full frame rate case at once. By my estimates this would be good for another 25% or so performance improvement because it would lower the required memory bandwidth for 2 frames of a full hd stream from 2x8MB read / 2x4MB write to 10MB read / 2x4MB write as most of the data read is the same and hence could be read from cache for the second frame.) Any ideas? Roland ------------------------------------------------------------------------------ Better than sec? Nothing is better than sec when it comes to monitoring Big Data applications. Try Boundary one-second resolution app monitoring today. Free. http://p.sf.net/sfu/Boundary-dev2dev _______________________________________________ xine-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/xine-devel
greedy2frame_sse2.diff
(text/x-patch, 1.2 KB)
diff -r 23f62fa05c72 src/post/deinterlace/plugins/greedy2frame.c
--- a/src/post/deinterlace/plugins/greedy2frame.c Wed Dec 21 21:23:21 2011 +0100
+++ b/src/post/deinterlace/plugins/greedy2frame.c Tue Apr 10 02:46:41 2012 +0200
@@ -38,15 +38,33 @@
#include "speedy.h"
#include "plugins.h"
-// debugging feature
-// output the value of mm4 at this point which is pink where we will weave
-// and green were we are going to bob
-// uncomment next line to see this
-//#define CHECK_BOBWEAVE
+/* debugging feature
+ * will show green or pink depending on weave/bob decision
+ */
+/*#define CHECK_BOBWEAVE*/
static int GreedyTwoFrameThreshold = 4;
static int GreedyTwoFrameThreshold2 = 8;
+#if defined ARCH_X86_64
+#include "greedy2frame_template2.c"
+
+static deinterlace_method_t greedy2framemethod =
+{
+ "Greedy 2-frame (DScaler)",
+ "Greedy2Frame",
+ 4,
+ MM_ACCEL_X86_SSE2,
+ 0,
+ 0,
+ 0,
+ 0,
+ DeinterlaceGreedy2Frame_SSE2,
+ 1,
+ NULL
+};
+
+#else
#define IS_SSE 1
#include "greedy2frame_template.c"
#undef IS_SSE
@@ -66,6 +84,8 @@
NULL
};
+#endif
+
deinterlace_method_t *greedy2frame_get_method( void )
{
return &greedy2framemethod;
greedy2frame_template2.c
(text/x-csrc, 10.6 KB)
/*****************************************************************************
** Copyright (c) 2000 John Adcock, Tom Barry, Steve Grimm All rights reserved.
** port copyright (c) 2003 Miguel Freitas
******************************************************************************
**
** This file is subject to the terms of the GNU General Public License as
** published by the Free Software Foundation. A copy of this license is
** included with this software distribution in the file COPYING. If you
** do not have a copy, you may obtain a copy by writing to the Free
** Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
**
** This software is distributed in the hope that it will be useful,
** but WITHOUT ANY WARRANTY; without even the implied warranty of
** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
** GNU General Public License for more details
******************************************************************************
** CVS Log
**
** Revision 1.10 2006/12/21 09:54:45 dgp85
** Apply the textrel patch from Gentoo, thanks to PaX team for providing it. The patch was applied and tested for a while in Gentoo and Pardus, and solves also Debian's problems with non-PIC code. If problems will arise, they'll be debugged.
**
** Revision 1.9 2006/02/04 14:06:29 miguelfreitas
** Enable AMD64 mmx/sse support in some plugins (tvtime, libmpeg2, goom...)
** patch by dani3l
**
** Revision 1.8 2005/06/05 16:00:06 miguelfreitas
** quite some hacks for gcc 2.95 compatibility
**
** Revision 1.7 2004/04/09 02:57:06 miguelfreitas
** tvtime deinterlacing algorithms assumed top_field_first=1
** top_field_first=0 (aka bottom_field_first) should now work as expected
**
** Revision 1.6 2004/02/12 20:53:31 mroi
** my gcc (partly 3.4 already) optimizes these away, because they are only used
** inside inline assembler (which the compiler does not recognize); so actually
** the code is wrong (the asm parts should list these as inputs), but telling
** the compiler to keep them is the easier fix
**
** Revision 1.5 2004/01/05 12:15:55 siggi
** wonder why Mike isn't complaining about C++ style comments, any more...
**
** Revision 1.4 2004/01/05 01:47:26 tmmm
** DOS/Win CRs are forbidden, verboten, interdit
**
** Revision 1.3 2004/01/02 20:53:43 miguelfreitas
** better MANGLE from ffmpeg
**
** Revision 1.2 2004/01/02 20:47:03 miguelfreitas
** my small contribution to the cygwin port ;-)
**
** Revision 1.1 2003/06/22 17:30:03 miguelfreitas
** use our own port of greedy2frame (tvtime port is currently broken)
**
** Revision 1.8 2001/11/23 17:18:54 adcockj
** Fixed silly and/or confusion
**
** Revision 1.7 2001/11/22 22:27:00 adcockj
** Bug Fixes
**
** Revision 1.6 2001/11/21 15:21:40 adcockj
** Renamed DEINTERLACE_INFO to TDeinterlaceInfo in line with standards
** Changed TDeinterlaceInfo structure to have history of pictures.
**
** Revision 1.5 2001/07/31 06:48:33 adcockj
** Fixed index bug spotted by Peter Gubanov
**
** Revision 1.4 2001/07/13 16:13:33 adcockj
** Added CVS tags and removed tabs
**
*****************************************************************************/
/*
* This is the implementation of the Greedy 2-frame deinterlace algorithm
* described in DI_Greedy2Frame.c. It's in a separate file so we can compile
* variants for different CPU types; most of the code is the same in the
* different variants.
*/
/****************************************************************************
** Field 1 | Field 2 | Field 3 | Field 4 |
** T0 | | T1 | |
** | M0 | | M1 |
** B0 | | B1 | |
*/
typedef unsigned int uint128_t __attribute__((mode(TI)));
static void DeinterlaceGreedy2Frame_SSE2(uint8_t *output, int outstride,
deinterlace_frame_data_t *data,
int bottom_field, int second_field,
int width, int height )
{
#if defined(ARCH_X86) || defined(ARCH_X86_64)
int Line;
int stride = width * 2;
register uint8_t* M1;
register uint8_t* M0;
register uint8_t* T1;
register uint8_t* T0;
uint8_t* Dest = output;
register uint8_t* Dest2;
register uint8_t* Destc;
register int count;
/* Hmm??? */
#if defined(ARCH_X86_64)
uint64_t Pitch = stride * 2;
#else
uint32_t Pitch = stride * 2;
#endif
uint32_t LineLength = stride;
uint32_t PitchRest = Pitch - (LineLength >> 4)*16;
/* surely there is some way to do this which doesn't totally suck? */
uint128_t Mask128 = 0x7f7f7f7f7f7f7f7fll;
uint128_t GreedyTwoFrameThreshold128 = GreedyTwoFrameThreshold |
GreedyTwoFrameThreshold2 << 8 |
GreedyTwoFrameThreshold << 16 |
GreedyTwoFrameThreshold2 << 24;
Mask128 = Mask128 | Mask128 << 64;
GreedyTwoFrameThreshold128 = GreedyTwoFrameThreshold128 |
GreedyTwoFrameThreshold128 << 32 |
GreedyTwoFrameThreshold128 << 64 |
GreedyTwoFrameThreshold128 << 96;
if( second_field ) {
M1 = data->f0;
T1 = data->f0;
M0 = data->f1;
T0 = data->f1;
} else {
M1 = data->f0;
T1 = data->f1;
M0 = data->f1;
T0 = data->f2;
}
if( bottom_field ) {
M1 += stride;
T1 += 0;
M0 += stride;
T0 += 0;
} else {
M1 += Pitch;
T1 += stride;
M0 += Pitch;
T0 += stride;
xine_fast_memcpy(Dest, M1, LineLength);
Dest += outstride;
}
for (Line = 0; Line < (height / 2) - 1; ++Line)
{
/* Always use the most recent data verbatim. By definition it's correct
* (it'd be shown on an interlaced display) and our job is to fill in
* the spaces between the new lines.
*/
/* xine_fast_memcpy would be pretty pointless here as we load the same
* data anyway it's just one additional mov per loop...
* XXX I believe some cpus with sse2 (early A64?) only have one write
* buffer. Using movntdq with 2 different streams may have quite
* bad performance consequences on such cpus.
*/
Destc = Dest;
Dest += outstride;
Dest2 = Dest;
/* just rely on gcc not using xmm regs... */
do {
asm volatile(
"movdqa %0, %%xmm6 \n\t" // xmm6 = Mask
"pxor %%xmm7, %%xmm7 \n\t" // xmm7 = zero
: /* no output */
: "m" (Mask128) );
} while (0);
count = LineLength >> 4;
do {
asm volatile(
/* Figure out what to do with the scanline above the one we copy.
* See above for a description of the algorithm.
* weave if (weave(M) AND (weave(T) OR weave(B)))
*/
".align 16 \n\t"
"movdqa (%5), %%xmm1 \n\t" /* xmm1 = T1 */
"movdqa (%6), %%xmm0 \n\t" /* xmm0 = T0 */
"movdqa (%7,%5), %%xmm3 \n\t" /* xmm3 = B1 */
"movdqa (%7,%6), %%xmm2 \n\t" /* xmm2 = B0 */
/* calculate |T1-T0| keep T1 put result in xmm5 */
"movdqa %%xmm1, %%xmm5 \n\t"
"psubusb %%xmm0, %%xmm5 \n\t"
"psubusb %%xmm1, %%xmm0 \n\t"
"por %%xmm0, %%xmm5 \n\t"
"movdqa (%1), %%xmm0 \n\t" /* xmm0 = M1 */
/* T1 is data for line to copy */
"movntdq %%xmm1, %4 \n\t"
/* if |T1-T0| > Threshold we want 0 else dword minus one */
"psrlw $1, %%xmm5 \n\t"
"pand %%xmm6, %%xmm5 \n\t"
"pcmpgtb %3, %%xmm5 \n\t"
"pcmpeqd %%xmm7, %%xmm5 \n\t"
/* calculate |B1-B0| keep B1 put result in xmm4 */
"movdqa %%xmm3, %%xmm4 \n\t"
"psubusb %%xmm2, %%xmm4 \n\t"
"psubusb %%xmm3, %%xmm2 \n\t"
"por %%xmm2, %%xmm4 \n\t"
"movdqa (%2), %%xmm2 \n\t" /* xmm2 = M0 */
/* if |B1-B0| > Threshold we want 0 else dword minus one */
"psrlw $1, %%xmm4 \n\t"
"pand %%xmm6, %%xmm4 \n\t"
"pcmpgtb %3, %%xmm4 \n\t"
"pcmpeqd %%xmm7, %%xmm4 \n\t"
"prefetcht0 64(%7,%5) \n\t"
"prefetcht0 64(%7,%6) \n\t"
"por %%xmm4, %%xmm5 \n\t"
/* Average T1 and B1 so we can do interpolated bobbing if we bob
* onto T1 */
"pavgb %%xmm3, %%xmm1 \n\t" /* xmm1 = avg(T1,B1) */
"prefetcht0 64(%1) \n\t"
"prefetcht0 64(%2) \n\t"
/* make mm0 the average of M1 and M0 which should make weave
* look better when there is small amounts of movement */
"movdqa %%xmm2, %%xmm3 \n\t"
"pavgb %%xmm0, %%xmm3 \n\t" /* xmm3 = avg(M1,M0) */
/* calculate |M1-M0| put result in xmm4 */
"movdqa %%xmm0, %%xmm4 \n\t"
"psubusb %%xmm2, %%xmm4 \n\t"
"psubusb %%xmm0, %%xmm2 \n\t"
"por %%xmm2, %%xmm4 \n\t"
/* if |M1-M0| > Threshold we want 0 else dword minus one */
"psrlw $1, %%xmm4 \n\t"
"pand %%xmm6, %%xmm4 \n\t"
"pcmpgtb %3, %%xmm4 \n\t"
"pcmpeqd %%xmm7, %%xmm4 \n\t" /* do we want to bob */
"pand %%xmm5, %%xmm4 \n\t"
/* debugging feature
* output the value of xmm4 at this point which is pink where we will weave
* and green where we are going to bob
*/
#ifdef CHECK_BOBWEAVE
"movntdq %%xmm4, %0 \n\t"
#else
/* xmm4 now is 1 where we want to weave and 0 where we want to bob */
"pand %%xmm4, %%xmm3 \n\t"
"pandn %%xmm1, %%xmm4 \n\t"
"por %%xmm3, %%xmm4 \n\t"
"movntdq %%xmm4, %0 \n\t"
#endif
:
: "m" (*Dest2), "r" (M1), "r" (M0), "m" (GreedyTwoFrameThreshold128),
"m" (*Destc), "r" (T1), "r" (T0), "r" (Pitch) );
/* Advance to the next set of pixels. */
T1 += 16;
M1 += 16;
M0 += 16;
T0 += 16;
Dest2 += 16;
Destc += 16;
} while( --count );
Dest += outstride;
M1 += PitchRest;
T1 += PitchRest;
M0 += PitchRest;
T0 += PitchRest;
}
asm("sfence\n\t");
if( bottom_field )
{
xine_fast_memcpy(Dest, T1, stride);
Dest += outstride;
xine_fast_memcpy(Dest, M1, stride);
}
else
{
xine_fast_memcpy(Dest, T1, stride);
}
#endif
}