Hi, On Sun, Jun 25, 2017 at 3:27 PM, Michael Niedermayer <mich...@niedermayer.cc > 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 > <mich...@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 > Agreed. I'll leave it to James to decide on a final approach since it's his patch. :-). It sounds like we're both fine with either approach. > also potential future fate tests of simplemmx or other simd idcts > require there to be a way to select a specific idct output This is true, but don't forget that -cpuflags can also be used to cycle between the various -idct flavours. (This requires some trial and error, but it's not that many cpuflag-combinations.) Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
