[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 Mon, Jun 26, 2017 at 02:20:03PM +0200, James Darnley wrote:
> On 2017-06-25 21:27, Michael Niedermayer wrote:
> > 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:
> >>
> >>> 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 ...
> 
> I admit I haven't considered whether I should be using this with
> simplemmx.  I could change the code so that the new code isn't used for it.
> 

> If simplemmx is supposed to be its own algorithm available to anyone who
> might wish to use it then I think that an error should occur when MMX is
> not available.

yes, that would make sense


> 
> Since the current behaviour is to have simple as the catch-all fallback
> I will leave the code as is.  auto, simpleauto, simplemmx, and simple
> will now all use the new code.
> 
> We can discuss these points all you want but I intend to push the
> remaining 3 patches Soon(TM).  I have still not tried Gramner's
> suggestion so you have some time to object and block.
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- 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/20170627/10bd6e43/attachment.sig>