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.
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
