Hi Ronald, On 08.06.2015 23:18, Ronald S. Bultje wrote: > On Mon, Jun 8, 2015 at 5:08 PM, Andreas Cadhalpun < > andreas.cadhal...@googlemail.com> wrote: > >> On 07.06.2015 22:39, Michael Niedermayer wrote: >>> On Sun, Jun 07, 2015 at 04:27:42PM -0400, Ronald S. Bultje wrote: >>>> So what happens when you change mv_max/min to be integers (instead of >>>> int16_t) without further touching VP56mv, e.g. by making mv_max/min a >>>> VP8intminmaxmv (a newly created type just for this purpose, using int >>>> instead of int16_t)? >> >> This fixes the problem (but I chose the shorter name VP8intmv). >> A patch implementing this is attached. >> >> It seems libvpx is actually also using int for this type of information, >> although the implementation is different and instead of sv_{min,max}.{x,y} >> uses mb_to_{left,right,bottom,top}_edge (at least if I understood the code >> correctly). [1][2] >> >>> would this work with >>> clamp_mv() ? >>> it limits based in mv_max/min but writes into a VP56mv, so if these >>> out of 16bit limits can actually be reached then the output of >>> the clip would not fit in the 16bit destination ... >>> but maybe this doesnt occur, ive not deeply investigated >> >> I think it doesn't happen, but for safety I clip s->mv_{min,max}.{x,y} >> to int16_t range. >> >> Best regards, >> Andreas >> >> >> 1: >> https://sources.debian.net/src/libvpx/1.4.0-3/vp8/common/reconinter.c/?hl=340#L340 >> 2: >> https://sources.debian.net/src/libvpx/1.4.0-3/vp9/common/vp9_blockd.h/?hl=203#L203 > > > I think this should be ok, thanks for modifying it based on comments.
Pushed. Thanks a lot for the review. Best regards, Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel