FFmpeg-devel - PATCH - yadif port to libavfitler

This is a multi-part message in MIME format.
Hi guys,
Here is my first attempt to port yadif to libavfilter.
I chose the request-frame paradigm since the filter has delay.
What happens when no prev frame is available ? Original code seems to
read from
I'd like extensive review if possible and especially instructions on how
to retrieve cpu flags at runtime for MMX.
Also Michael, can you please check the calls to filter() ?
I'm also not sure how the field output mode should be supported.
Index: libavfilter/vf-yadif.c
==================================================================

This is a multi-part message in MIME format.
Patch updated, offseting the buffers as needed (please double check),
and memleak free :Questions still stands :)
The filter works without copying any input picture which is nice.
Index: libavfilter/vf-yadif.c
==================================================================

read from?
(it obviously should use spatial/directional interpolation for the very
first frame)
hmm, i guess we need to move libavcodec/x86/cpuid.c to libavutil
what do you want me to check exactly?
for every 2nd request-frame() we would just call our sources request-frame()
(the variant with calling 2 start/slice/end in one request-frame() might work
too but i thinks its less ideal)
does md5 match to mplayers ?
[...]
[...]
switching from fast-memcpy() (that works with SIMD) to memcpy() possibly looses speed
[...]
it should be trivial to support other yuv formats
other improvments that could be tried once its all in svn are to
try to pass draw-slice() calls through from the source and maybe to
pass get-video-buffer() through to avoid the every 2nd line memcpy.
also if bobby could take a look at this patch it would be quite good
i likely miss some issues.
[...]
Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners.

Hi Michael,
The parameters in the filter() call. I'm not sure I fully understand the
logic width fields and parity. Should it be 1 ^ tff if we output only
frames for now ?
Is there a md5 in ffmpeg available already ? Otherwise I can cook
something up.
Or can I just enable it for a specific gcc version to get it into svn
quickly ?
Do we have a replacement ?
Well, with the delay it might be tricky.
In short what's required before svn inclusion ?
[...]

parity selects if we interpolate even or odd lines iirc so that should stay
(that is from memory havnt rechecked the code)
hmm, seems not, i am just spotting crcenc.c which uses adler32 and our
md5 utility code
i wouldnt mind but mans will want a proper configure check i think
i dont think we do.
cpuid + matching md5 to mplayer
(id also like to see double frame output to work if possible but if thats
a problem we could look into it later)
and if fast-memcpy isnt ported we need a todo/fixme somewhere so we dont
forget it
[...]
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.

Michael Niedermayer writes:
Which version was the first to support that syntax?
Or use yasm.

On Sun, May 23, 2010 at 01:22:11PM +0200, Michael Niedermayer wrote:
Huh? There is md5 code in libavutil of course.
If you want something like framecrc with md5, there's an old patch from me on
the list which got stuck because some people wanted it to do everything and wash
the cat and I couldn't be bothered.

i dont remember 3.something id guess
configure trying to compile a 2 line asm() should need to know that though
i prefer if the code is not changed from what is in mplayer as maintaince
burden otherwise becomes unreasonable
if we add libavfilter support to mplayer in a form that has no performance
penalty and that works with the same old syntax then we could drop yadif
from libmpcodecs.
[...]
The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"

On 5/23/10 4:22 AM, Michael Niedermayer wrote:
Thanks to Reimar we have one now, framemd5 should be applied asap, it's
very useful when porting filters.
md5 matches, cpuid is a bit complicated to port since MM-FLAGS are
defined in avcodec.h
Should memcpy be ported to libavutil ?

Different levels of nontemporal hinting should be useful for improving cache behavior in specific cases, but Darwin memcpy has about the same tuning as mplayer's and is more up to date.
Also IIRC mplayer svn's fastmemcpybench doesn't try to use the copied data in the fastmemcpy bench, so whatever is the most nontemporal always wins, and you might not always want that.

is faster it should be used
and if you think you can improve it (with additional temporality amount or
therwise), a patch on mplayer-dev is likely welcome
[...]
Observe your enemies, for they first find out your faults.

[...]
where is the problem with moving them to some header in avutil ?
if its faster than normal memcpy in yadif (which i assumed) then yes
[...]
I do not agree with what you have to say, but I'll defend to the death your
right to say it.

Michael Niedermayer writes:
Make that a new header only for cpuid stuff.

This is a multi-part message in MIME format.
All right, first attempt attached.
Index: libavcodec/avcodec.h
================================================================== #define LIBAVCODEC-VERSION-MAJOR 52
-#define LIBAVCODEC-VERSION-MINOR 68
+#define LIBAVCODEC-VERSION-MINOR 69
#define LIBAVCODEC-VERSION-MICRO 3
#define LIBAVCODEC-VERSION-INT AV-VERSION-INT(LIBAVCODEC-VERSION-MAJOR,
@@ -1620,6 +1620,7 @@
* result into program crash.)
*/
unsigned dsp-mask;
+#if LIBAVUTIL-VERSION-INT < AV-VERSION-INT(50,17,0)
#define FF-MM-FORCE 0x80000000 /* Force usage of selected flags (OR) */
/* lower 16 bits - CPU features */
#define FF-MM-MMX 0x0001 ///< standard MMX
@@ -1637,6 +1638,7 @@
#define FF-MM-SSE42 0x0200 ///< Nehalem SSE4.2 functions
#define FF-MM-IWMMXT 0x0100 ///< XScale IWMMXT
#define FF-MM-ALTIVEC 0x0001 ///< standard AltiVec
+#endif
/**
* bits per sample/pixel from the demuxer (needed for huffyuv).
Index: libavutil/Makefile
==================================================================+OBJS-$(ARCH-X86) += x86/cpuid.o
+
TESTPROGS = adler32 aes base64 crc des lls md5 pca sha softfloat tree
TESTPROGS-$(HAVE-LZO1X-999-COMPRESS) += lzo
Index: libavutil/cpuid.h
================================================================== MMX-OBJS-$(CONFIG-FFT) += x86/fft.o
-OBJS-$(HAVE-MMX) += x86/cpuid.o
- x86/dnxhd-mmx.o
+OBJS-$(HAVE-MMX) += x86/dnxhd-mmx.o
x86/dsputil-mmx.o
x86/fdct-mmx.o
x86/idct-mmx-xvid.o
Index: libavutil/x86/cpuid.c
================================================================== #include
#include "libavutil/x86-cpu.h"
-#include "libavcodec/dsputil.h"
+#include "libavutil/cpuid.h"
#undef printf
Index: libavcodec/dsputil.h
================================================================== #include "libavutil/intreadwrite.h"
+#include "libavutil/cpuid.h"
#include "avcodec.h"
@@ -624,11 +625,6 @@
*/
#define emms-c()
-/* should be defined by architectures supporting
- one or more MultiMedia extension */
-int mm-support(void);
-extern int mm-flags;
-
void dsputil-init-alpha(DSPContext* c, AVCodecContext *avctx);
void dsputil-init-arm(DSPContext* c, AVCodecContext *avctx);
void dsputil-init-bfin(DSPContext* c, AVCodecContext *avctx);

Michael Niedermayer writes:
Why not start with a version without cpuid?
Carl Eugen

remaining code ok with me
[...]
If you really think that XML is the answer, then you definitly missunderstood
the question

This is a multi-part message in MIME format.
yadif updated as well.
Issues are named args in asm
yadif should be compiled only when gpl is enabled.
Index: libavutil/Makefile
==================================================================+OBJS-$(ARCH-X86) += x86/cpuid.o
+
TESTPROGS = adler32 aes base64 crc des lls md5 pca sha softfloat tree
TESTPROGS-$(HAVE-LZO1X-999-COMPRESS) += lzo
Index: libavutil/cpuid.h
==================================================================+#include "libavutil/cpuid.h"
#include "libavutil/intreadwrite.h"
#include "avcodec.h"
@@ -624,11 +625,6 @@
*/
#define emms-c()
-/* should be defined by architectures supporting
- one or more MultiMedia extension */
-int mm-support(void);
-extern int mm-flags;
-
void dsputil-init-alpha(DSPContext* c, AVCodecContext *avctx);
void dsputil-init-arm(DSPContext* c, AVCodecContext *avctx);
void dsputil-init-bfin(DSPContext* c, AVCodecContext *avctx);
Index: libavcodec/avcodec.h
================================================================== #define LIBAVCODEC-VERSION-MAJOR 52
-#define LIBAVCODEC-VERSION-MINOR 74
-#define LIBAVCODEC-VERSION-MICRO 1
+#define LIBAVCODEC-VERSION-MINOR 75
+#define LIBAVCODEC-VERSION-MICRO 0
#define LIBAVCODEC-VERSION-INT AV-VERSION-INT(LIBAVCODEC-VERSION-MAJOR,
LIBAVCODEC-VERSION-MINOR,
@@ -1625,6 +1625,7 @@
* result into program crash.)
*/
unsigned dsp-mask;
+#if LIBAVUTIL-VERSION-INT < AV-VERSION-INT(50,17,0)
#define FF-MM-FORCE 0x80000000 /* Force usage of selected flags (OR) */
/* lower 16 bits - CPU features */
#define FF-MM-MMX 0x0001 ///< standard MMX
@@ -1642,6 +1643,7 @@
#define FF-MM-SSE42 0x0200 ///< Nehalem SSE4.2 functions
#define FF-MM-IWMMXT 0x0100 ///< XScale IWMMXT
#define FF-MM-ALTIVEC 0x0001 ///< standard AltiVec
+#endif
/**
* bits per sample/pixel from the demuxer (needed for huffyuv).
Index: libavcodec/x86/Makefile
================================================================== MMX-OBJS-$(CONFIG-FFT) += x86/fft.o
-OBJS-$(HAVE-MMX) += x86/cpuid.o
- x86/dnxhd-mmx.o
+OBJS-$(HAVE-MMX) += x86/dnxhd-mmx.o
x86/dsputil-mmx.o
x86/fdct-mmx.o
x86/idct-mmx-xvid.o
Index: libavfilter/vf-yadif.c
==================================================================

On Sun, Jun 06, 2010 at 02:28:17PM -0700, Baptiste Coudurier wrote:
add "yadif-filter-deps=gpl" to configure
Since yadif will be built by default die-license-disabled doesn't work.
Janne

header?
yadif itself should be ok if tested
[...]
If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.

On 6/10/10 3:34 AM, Michael Niedermayer wrote:
libavcodec/x86 to libavutil, like the avutil Makefile diff shows.

[...]
Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them.