diff mbox series

[FFmpeg-devel] libswscale/swscale.c: Clarify what exactly 'data is not aligned' to

Message ID trinity-4c37d1c6-f8bf-4871-9f3d-bfc8c6fc956a-1608403839141@3c-app-gmx-bs71
State New
Headers show
Series [FFmpeg-devel] libswscale/swscale.c: Clarify what exactly 'data is not aligned' to | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Franziska Thul Dec. 19, 2020, 6:50 p.m. UTC 
libswscale/swscale.c emits a Warning that 'data is not aligned', but doesn't explain
why, leaving users without any clue on how to address this issue.
This patch simply adds that data is not aligned 'to 16 pixel boundaries'.

Idealy, the warning would say which values are not aligned, too, but since the
variables' uses are probably explained elsewhere, I could only speculate.

---
 libswscale/swscale.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--
2.20.1

Comments

Michael Niedermayer Dec. 21, 2020, 11:18 p.m. UTC | #1 
On Sat, Dec 19, 2020 at 07:50:39PM +0100, Franziska Thul wrote:
> libswscale/swscale.c emits a Warning that 'data is not aligned', but doesn't explain
> why, leaving users without any clue on how to address this issue.
> This patch simply adds that data is not aligned 'to 16 pixel boundaries'.
> 
> Idealy, the warning would say which values are not aligned, too, but since the
> variables' uses are probably explained elsewhere, I could only speculate.
> 
> ---
>  libswscale/swscale.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> index 9cb7e8f6ac..69349a7349 100644
> --- a/libswscale/swscale.c
> +++ b/libswscale/swscale.c
> @@ -311,8 +311,7 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>          static int warnedAlready = 0; // FIXME maybe move this into the context
>          if (flags & SWS_PRINT_INFO && !warnedAlready) {
>              av_log(c, AV_LOG_WARNING,
> -                   "Warning: dstStride is not aligned!\n"
> -                   "         ->cannot do aligned memory accesses anymore\n");
> +                   "Warning: dstStride is not aligned to a 16 pixel boundary! Cannot do aligned memory accesses anymore.\n");
>              warnedAlready = 1;
>          }
>      }
> @@ -325,7 +324,7 @@ static int swscale(SwsContext *c, const uint8_t *src[],
>          static int warnedAlready=0;
>          int cpu_flags = av_get_cpu_flags();
>          if (HAVE_MMXEXT && (cpu_flags & AV_CPU_FLAG_SSE2) && !warnedAlready){
> -            av_log(c, AV_LOG_WARNING, "Warning: data is not aligned! This can lead to a speed loss\n");
> +            av_log(c, AV_LOG_WARNING, "Warning: data is not aligned to a 16 pixel boundary! This can lead to a speed loss.\n");
>              warnedAlready=1;
>          }

all the alignment is by byte not by pixel

thx

[...]
Franziska Thul Dec. 27, 2020, 8:36 a.m. UTC | #2 
Michael Niedermayer wrote:

> > libswscale/swscale.c emits a Warning that 'data is not aligned', but doesn't explain
> > why, leaving users without any clue on how to address this issue.
> > This patch simply adds that data is not aligned 'to 16 pixel boundaries'.

> all the alignment is by byte not by pixel

Well, another point clarified :) I based my assumption of pixel alignment on try-and-error
with ffmpeg's parameters, and while scaling an image the user is, of course, dealing with
pixels, not bytes.
Not sure if mentioning bytes in that warning might cause more confusion than it helps. If
"bytes" or "pixels" doesn't matter from the user's perspective, I'd stick with the latter,
even if it's not exactly right regarding the internal workings of ffmpeg.

In any case, the 16 is important.

Thanks, Franziska
Carl Eugen Hoyos Dec. 27, 2020, 12:34 p.m. UTC | #3 
> Am 27.12.2020 um 09:36 schrieb Franziska Thul <franziskavon1999@gmx.de>:
> 
> If "bytes" or "pixels" doesn't matter from the user's perspective, I'd stick with the latter, even if it's not exactly right regarding the internal workings of ffmpeg.

It does matter because while data has to be 16-bytes aligned, this is not true for all pixels that libswscale deals with.

Please send a new patch, Carl Eugen
diff mbox series

Patch

diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 9cb7e8f6ac..69349a7349 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -311,8 +311,7 @@  static int swscale(SwsContext *c, const uint8_t *src[],
         static int warnedAlready = 0; // FIXME maybe move this into the context
         if (flags & SWS_PRINT_INFO && !warnedAlready) {
             av_log(c, AV_LOG_WARNING,
-                   "Warning: dstStride is not aligned!\n"
-                   "         ->cannot do aligned memory accesses anymore\n");
+                   "Warning: dstStride is not aligned to a 16 pixel boundary! Cannot do aligned memory accesses anymore.\n");
             warnedAlready = 1;
         }
     }
@@ -325,7 +324,7 @@  static int swscale(SwsContext *c, const uint8_t *src[],
         static int warnedAlready=0;
         int cpu_flags = av_get_cpu_flags();
         if (HAVE_MMXEXT && (cpu_flags & AV_CPU_FLAG_SSE2) && !warnedAlready){
-            av_log(c, AV_LOG_WARNING, "Warning: data is not aligned! This can lead to a speed loss\n");
+            av_log(c, AV_LOG_WARNING, "Warning: data is not aligned to a 16 pixel boundary! This can lead to a speed loss.\n");
             warnedAlready=1;
         }
     }