"Ronald S. Bultje" <[email protected]> writes: > Hi, > > On Fri, Nov 11, 2011 at 6:46 AM, Kostya Shishkov > <[email protected]> wrote: >> On Fri, Nov 11, 2011 at 06:44:10AM -0800, Ronald S. Bultje wrote: >>> 2011/11/11 Måns Rullgård <[email protected]>: >>> > "Ronald S. Bultje" <[email protected]> writes: >>> >> 2011/11/10 Måns Rullgård <[email protected]>: >>> >>> "Ronald S. Bultje" <[email protected]> writes: >>> >>>> 2011/11/10 Måns Rullgård <[email protected]>: >>> >>>>> Sean McGovern <[email protected]> writes: >>> >>>>>> valgrind is not fond of the pointer math in hScale_altivec_real(), >>> >>>>> >>> >>>>> This is a weird description. Valgrind never complains about pointer >>> >>>>> maths as such, only about actual memory accesses. I'm guessing it has >>> >>>>> to do with loading from an unaligned array, which will overread the >>> >>>>> end >>> >>>>> up to a 16-byte aligned position. >>> >>>> >>> >>>> The array is aligned. Doesn't it just read 32 pixels at once, i.e. the >>> >>>> FFALIGN(width, 16) should be changed to 32? >>> >>> >>> >>> The function is doing unaligned loads. Maybe it doesn't need to. >>> >> >>> >> I see. OK, patch is fine then. Maybe add a comment that the +16 is to >>> >> account for buggy altivec overreads. >>> > >>> > They are not buggy. It's the only way to read unaligned data in >>> > altivec, so unless you consider altivec itself buggy, the code is >>> > correct provided it needs to handle unaligned buffers. >>> >>> But it doesn't need to; convertData is guaranteed to be aligned. It >>> was allocated using av_malloc(). >> >> that doesn't mean data cannot be read with offset not multiple of 16 from it > > From what I remember, the invalid read was always in: > > vector unsigned char src_v0 = vec_ld(srcPos, src); > vector unsigned char src_v1 = vec_ld(srcPos + 16, src); > > The second line, specifically. That is always an offset multiple of 16.
Please go and read about how altivec unaligned loads are done. If src+srcPos is always 16-byte aligned, the second line is not needed at all. If not, the overread is necessary. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
