[FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv

[Kacper Michajlow]

On Sun, 2 Jun 2024 at 23:17, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>
> Hi,
>
> On Sun, Jun 2, 2024 at 9:12 AM James Almer <jamrial at gmail.com> wrote:
>
> > On 6/2/2024 10:06 AM, James Almer wrote:
> > > On 6/2/2024 9:14 AM, Kacper Michajłow wrote:
> > >> Fixes runtime error: member access within misaligned address
> > >> <addr> for type 'av_alias64', which requires 8 byte alignment.
> > >>
> > >> VP9mv is aligned to 4 bytes, so instead doing 8 bytes clear, let's do
> > >> 2 times 4 bytes.
> > >>
> > >> Signed-off-by: Kacper Michajłow <kasper93 at gmail.com>
> > >> ---
> > >>   libavcodec/vp9mvs.c | 3 ++-
> > >>   1 file changed, 2 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/libavcodec/vp9mvs.c b/libavcodec/vp9mvs.c
> > >> index b706d1660f..790cf629a6 100644
> > >> --- a/libavcodec/vp9mvs.c
> > >> +++ b/libavcodec/vp9mvs.c
> > >> @@ -294,7 +294,8 @@ void ff_vp9_fill_mv(VP9TileData *td, VP9mv *mv,
> > >> int mode, int sb)
> > >>       VP9Block *b = td->b;
> > >>       if (mode == ZEROMV) {
> > >> -        AV_ZERO64(mv);
> > >> +        AV_ZERO32(&mv[0]);
> > >> +        AV_ZERO32(&mv[1]);
> > >>       } else {
> > >>           int hp;
> > >
> > > IMO just move mv in VP9Block to the top of the struct. That will make
> > > sure it's aligned to at the very least 16 byte (Since it's av_malloc'd).
> >
> > Actually nevermind, VP9mv has two int16_t and given what's passed to
> > ff_vp9_fill_mv() it's not enough.
> >
>
> Do compilers on relevant platforms convert this to a single 64bit
> (unaligned) zero-move? Otherwise, we may want an unaligned AV_ZERO64() so
> as to not slow down platforms supporting unaligned writes.

Yes, exactly, compilers do that. I've checked before sending this
patch if it doesn't do something overly silly.

You can play around here to see, I've extracted relevant part
https://godbolt.org/z/K4d7Ejb1P

- Kacper