Diego Biurrun <[email protected]> writes:
> On Thu, Feb 02, 2012 at 11:48:34AM -0800, Ronald S. Bultje wrote:
>> [...]
>
> Patch should be OK if it satisfies Mans, here's some small stuff I noticed
> that you could change before pushing.
>
>> --- a/configure
>> +++ b/configure
>> @@ -254,6 +254,8 @@ Developer options (useful when working on Libav itself):
>> --samples=PATH location of test samples for FATE, if not set use
>> \$FATE_SAMPLES at make invocation time.
>> + --enable-xmm-clobber-test check XMM registers for clobbering (Win64-only;
>> + should be used only for debugging purposes)
>> @@ -991,6 +993,7 @@ CONFIG_LIST="
>> version3
>> + xmm_clobber_test
>> x11grab
>
> I think this could be shortened further to "xmm_clobber".
I don't think so. That suggests that the option enables clobbering of
xmm, which whatever it means, isn't what this does.
>> --- a/libavcodec/x86/Makefile
>> +++ b/libavcodec/x86/Makefile
>> @@ -74,3 +74,4 @@ OBJS-$(HAVE_MMX) +=
>> x86/dsputil_mmx.o \
>> x86/mpegvideo_mmx.o \
>> x86/simple_idct_mmx.o \
>>
>> +OBJS-$(CONFIG_XMM_CLOBBER_TEST) += x86/w64xmmtest.o
>
> Please move this to line 60.
Why there of all places?
>> +#define testxmmclobbers(func, ctx, ...) \
>> + uint64_t xmm[2][10][2]; \
>> + int ret; \
>> + storexmmregs(xmm[0]); \
>> + ret = __real_ ## func(ctx, __VA_ARGS__); \
>> + storexmmregs(xmm[1]); \
>> + if (memcmp(xmm[0], xmm[1], sizeof(xmm[0]))) { \
>> + int i; \
>> + av_log(ctx, AV_LOG_ERROR, \
>> + "XMM REGS CLOBBERED IN %s!\n", #func); \
>> + for (i = 0; i < 10; i ++) \
>> + if (xmm[0][i][0] != xmm[1][i][0] || \
>> + xmm[0][i][1] != xmm[1][i][1]) { \
>> + av_log(ctx, AV_LOG_ERROR, \
>> + "xmm%-2d = %016"PRIx64"%016"PRIx64"\n", \
>> + 6 + i, av_bswap64(xmm[0][i][0]), \
>> + av_bswap64(xmm[0][i][1])); \
>> + av_log(ctx, AV_LOG_ERROR, \
>> + " -> %016"PRIx64"%016"PRIx64"\n", \
>
> Indentation is off by one character.
Intentionally.
--
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
