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

[Michael Niedermayer]

On Sat, Jun 24, 2017 at 06:30:26PM -0400, Ronald S. Bultje wrote:
> 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...

I didnt think about meaning, rather more about practice.
if someone reports any issue using "simplemmx" and bitexact and
that fails to be reproduced it could be confusing.
This is especially plausible when the bug is not idct rounding but
a bug in a later stage just triggered by specific output from the idct

also potential future fate tests of simplemmx or other simd idcts
require there to be a way to select a specific idct output

no strong oppinion on this ...

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170625/00048de9/attachment.sig>