On 2016-09-29 17:52:01 +0200, Diego Biurrun wrote: > On Thu, Sep 29, 2016 at 02:21:17PM +0300, Martin Storsjö wrote: > > On Tue, 20 Sep 2016, Diego Biurrun wrote: > > > > > This avoids SIMD-optimized functions having to sign-extend their > > > stride argument manually to be able to do pointer arithmetic. > > > --- > > > libavcodec/arm/rv34dsp_neon.S | 4 +-- > > > libavcodec/arm/rv40dsp_neon.S | 4 +-- > > > libavcodec/rv30.c | 4 +-- > > > libavcodec/rv30dsp.c | 64 > > > +++++++++++++++++++++++++++++++++++-------- > > > libavcodec/rv34.c | 10 ++++--- > > > libavcodec/rv34.h | 2 +- > > > libavcodec/rv40.c | 2 +- > > > libavcodec/rv40dsp.c | 18 +++++++----- > > > libavcodec/x86/rv34dsp.asm | 4 +-- > > > libavcodec/x86/rv40dsp.asm | 11 +++----- > > > libavcodec/x86/rv40dsp_init.c | 4 +-- > > > 11 files changed, 85 insertions(+), 42 deletions(-) > > > --- a/libavcodec/rv40dsp.c > > > +++ b/libavcodec/rv40dsp.c > > > @@ -34,7 +34,8 @@ > > > > > > --- a/libavcodec/x86/rv40dsp.asm > > > +++ b/libavcodec/x86/rv40dsp.asm > > > @@ -77,14 +77,11 @@ SECTION .text > > > ;----------------------------------------------------------------------------- > > > ; subpel MC functions: > > > ; > > > -; void ff_[put|rv40]_rv40_qpel_[h|v]_<opt>(uint8_t *dst, int deststride, > > > -; uint8_t *src, int srcstride, > > > -; int len, int m); > > > +; void ff_[put|avg]_rv40_qpel_[h|v]_<opt>(uint8_t *dst, ptrdiff_t > > > deststride, > > > +; uint8_t *src, ptrdiff_t > > > srcstride, > > > +; int len, ptrdiff_t m); > > > ;---------------------------------------------------------------------- > > > %macro LOAD 2 > > > -%if WIN64 > > > - movsxd %1q, %1d > > > -%endif > > > %ifdef PIC > > > add %1q, picregq > > > %else > > > @@ -438,7 +435,7 @@ FILTER_SSSE3 avg > > > > > > %endmacro > > > > > > -; void ff_rv40_weight_func_%1(uint8_t *dst, uint8_t *src1, uint8_t > > > *src2, int w1, int w2, int stride) > > > +; void ff_rv40_weight_func_%1(uint8_t *dst, uint8_t *src1, uint8_t > > > *src2, int w1, int w2, ptrdiff_t stride) > > > ; %1=size %2=num of xmm regs > > > ; The weights are FP0.14 notation of fractions depending on pts. > > > ; For timebases without rounding error (i.e. PAL), the fractions > > > > So, the only existing sign extension this actually changes is for the "int > > m" parameter. I don't see where the existing code does sign extension for > > srcstride nor dststride anywhere. Isn't that a latent bug, that you're > > fixing silently? > > All of the int strides are latent bugs. This file only does sign extension > for WIN64, for one function parameter. Possibly that was the only > environment that triggered an actual, visible bug, I don't know.
the difference is that the m parameter is in some macro instances negative. I'm not sure why it made a difference on win64 but it relies on non windows 64-bit systems that the parameter is sign extended to 64-bit in the register. > > In that case, please first explicitly fix the bug by introducing the right > > sign extensions (which is cherrypickable to release branches), then remove > > them in this patch. > > This sounds like overkill to me as we don't know if any real-world > samples are affected. If any such cases creep up I'll gladly add the > necessary sign extension for release branches. Please fix this in a separate patch. It differs from the other stride ptrdiff_t changes and affects only x86. Janne _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
