* [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv @ 2024-06-02 12:14 Kacper Michajłow 2024-06-02 13:06 ` James Almer 0 siblings, 1 reply; 7+ messages in thread From: Kacper Michajłow @ 2024-06-02 12:14 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Kacper Michajłow 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@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; -- 2.43.0 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv 2024-06-02 12:14 [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv Kacper Michajłow @ 2024-06-02 13:06 ` James Almer 2024-06-02 13:11 ` James Almer 0 siblings, 1 reply; 7+ messages in thread From: James Almer @ 2024-06-02 13:06 UTC (permalink / raw) To: ffmpeg-devel 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@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). _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv 2024-06-02 13:06 ` James Almer @ 2024-06-02 13:11 ` James Almer 2024-06-02 21:16 ` Ronald S. Bultje 0 siblings, 1 reply; 7+ messages in thread From: James Almer @ 2024-06-02 13:11 UTC (permalink / raw) To: ffmpeg-devel 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@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv 2024-06-02 13:11 ` James Almer @ 2024-06-02 21:16 ` Ronald S. Bultje 2024-06-02 22:43 ` Kacper Michajlow 0 siblings, 1 reply; 7+ messages in thread From: Ronald S. Bultje @ 2024-06-02 21:16 UTC (permalink / raw) To: FFmpeg development discussions and patches Hi, On Sun, Jun 2, 2024 at 9:12 AM James Almer <jamrial@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@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. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv 2024-06-02 21:16 ` Ronald S. Bultje @ 2024-06-02 22:43 ` Kacper Michajlow 2024-06-02 23:05 ` Ronald S. Bultje 0 siblings, 1 reply; 7+ messages in thread From: Kacper Michajlow @ 2024-06-02 22:43 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, 2 Jun 2024 at 23:17, Ronald S. Bultje <rsbultje@gmail.com> wrote: > > Hi, > > On Sun, Jun 2, 2024 at 9:12 AM James Almer <jamrial@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@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv 2024-06-02 22:43 ` Kacper Michajlow @ 2024-06-02 23:05 ` Ronald S. Bultje 2024-08-06 22:24 ` Michael Niedermayer 0 siblings, 1 reply; 7+ messages in thread From: Ronald S. Bultje @ 2024-06-02 23:05 UTC (permalink / raw) To: FFmpeg development discussions and patches Hi, On Sun, Jun 2, 2024 at 6:43 PM Kacper Michajlow <kasper93@gmail.com> wrote: > On Sun, 2 Jun 2024 at 23:17, Ronald S. Bultje <rsbultje@gmail.com> wrote: > > > > Hi, > > > > On Sun, Jun 2, 2024 at 9:12 AM James Almer <jamrial@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@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 > Thanks for checking, patch is fine with me. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv 2024-06-02 23:05 ` Ronald S. Bultje @ 2024-08-06 22:24 ` Michael Niedermayer 0 siblings, 0 replies; 7+ messages in thread From: Michael Niedermayer @ 2024-08-06 22:24 UTC (permalink / raw) To: FFmpeg development discussions and patches [-- Attachment #1.1: Type: text/plain, Size: 2730 bytes --] On Sun, Jun 02, 2024 at 07:05:37PM -0400, Ronald S. Bultje wrote: > Hi, > > On Sun, Jun 2, 2024 at 6:43 PM Kacper Michajlow <kasper93@gmail.com> wrote: > > > On Sun, 2 Jun 2024 at 23:17, Ronald S. Bultje <rsbultje@gmail.com> wrote: > > > > > > Hi, > > > > > > On Sun, Jun 2, 2024 at 9:12 AM James Almer <jamrial@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@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 > > > > Thanks for checking, patch is fine with me. will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you drop bombs on a foreign country and kill a hundred thousand innocent people, expect your government to call the consequence "unprovoked inhuman terrorist attacks" and use it to justify dropping more bombs and killing more people. The technology changed, the idea is old. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-08-06 22:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-06-02 12:14 [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv Kacper Michajłow 2024-06-02 13:06 ` James Almer 2024-06-02 13:11 ` James Almer 2024-06-02 21:16 ` Ronald S. Bultje 2024-06-02 22:43 ` Kacper Michajlow 2024-06-02 23:05 ` Ronald S. Bultje 2024-08-06 22:24 ` Michael Niedermayer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git