[FFmpeg-devel] [PATCH 10/11] avcodec/x86: add an 8-bit simple IDCT function based on the x86-64 high depth functions

Ronald S. Bultje rsbultje at gmail.com
Sun Jun 25 01:30:26 EEST 2017


Hi,

On Sat, Jun 24, 2017 at 3:27 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Mon, Jun 19, 2017 at 05:11:03PM +0200, James Darnley wrote:
> > Includes add/put functions
> >
> > Rounding contributed by Ronald S. Bultje
> > ---
> >  libavcodec/tests/x86/dct.c                |  2 +
> >  libavcodec/x86/idctdsp_init.c             | 23 ++++++++
> >  libavcodec/x86/simple_idct.h              |  9 +++
> >  libavcodec/x86/simple_idct10.asm          | 92
> +++++++++++++++++++++++++++++++
> >  libavcodec/x86/simple_idct10_template.asm |  6 +-
> >  5 files changed, 130 insertions(+), 2 deletions(-)
>
> Sorry for the delay, testing this took me a bit longer than expected
> as many files change slightly and looking at differences manually
> is manual work ...
>

Understood, and thanks for taking the time to do the testing.


> This patch changes the default IDCT on x86(64), which is intended IIUC
> It also changes the IDCT when simplemmx is set
>
> but on x86-32 simplemmx does after this patch not produce the same
> result as simplemmx on x86-64.
>
> iam not sure but
> maybe the changed code should enable on FF_IDCT_SIMPLE instead of
> FF_IDCT_SIMPLEMMX ?
> whats your oppinion on this ?
> the next patch would add FF_IDCT_SIMPLE but it also leaves
> FF_IDCT_SIMPLEMMX


 That's a good point, I also considered that question (not so much the
32bit vs. 64bit, but the mmx vs. sse2). The question is basically what
simplemmx means. Is it the exact binary result of the mmx function? Or is
it a way of saying "almost simple, but with some rounding diffs because
mmx"?

If the second, then simple is a superset of simplemmx. If the first, then
we should remove simplemmx from the list of "supported" idcts for the
sse2/avx functions. I have no preference (I assumed it meant the first),
but if you'd prefer to use the second meaning, then that's an easy
modification to make and it won't practically have any impact for most use
cases I think...

Ronald


More information about the ffmpeg-devel mailing list