Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes

[Michael Niedermayer]

On Tue, Oct 24, 2023 at 07:13:14AM -0400, Leo Izen wrote:
> Various parts of this file are restructured slightly for readability,
> such as spacing in arithmetic operations, and putting `if (ret < 0)`
> clauses on separate lines.
> 
> Signed-off-by: Leo Izen 
> ---
>  libavcodec/pngdec.c | 241 +++-
>  1 file changed, 124 insertions(+), 117 deletions(-)

This produces different output for multiple files
I think andreas already found some spot(s) in the code that are not
style changes, just want to confirm something in ths leads to actual differences

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes

[Andreas Rheinhardt]

Leo Izen:
> Various parts of this file are restructured slightly for readability,
> such as spacing in arithmetic operations, and putting `if (ret < 0)`
> clauses on separate lines.
> 
> Signed-off-by: Leo Izen 
> ---
>  libavcodec/pngdec.c | 241 +++-
>  1 file changed, 124 insertions(+), 117 deletions(-)
> 
> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index d812ffd348..93b067a9be 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -128,7 +128,7 @@ static const uint8_t png_pass_dsp_ymask[NB_PASSES] = {
>  
>  /* Mask to determine which pixels to overwrite while displaying */
>  static const uint8_t png_pass_dsp_mask[NB_PASSES] = {
> -0xff, 0x0f, 0xff, 0x33, 0xff, 0x55, 0xff
> +0xff, 0x0f, 0xff, 0x33, 0xff, 0x55, 0xff,
>  };
>  
>  /* NOTE: we try to construct a good looking image at each pass. width
> @@ -152,7 +152,7 @@ static void png_put_interlaced_row(uint8_t *dst, int 
> width,
>  j = (x & 7);
>  if ((dsp_mask << j) & 0x80) {
>  b = (src[src_x >> 3] >> (7 - (src_x & 7))) & 1;
> -dst[x >> 3] &= 0xFF7F>>j;
> +dst[x >> 3] &= 0xff7f >> j;
>  dst[x >> 3] |= b << (7 - j);
>  }
>  if ((mask << j) & 0x80)
> @@ -165,8 +165,8 @@ static void png_put_interlaced_row(uint8_t *dst, int 
> width,
>  int j2 = 2 * (x & 3);
>  j = (x & 7);
>  if ((dsp_mask << j) & 0x80) {
> -b = (src[src_x >> 2] >> (6 - 2*(src_x & 3))) & 3;
> -dst[x >> 2] &= 0xFF3F>>j2;
> +b = (src[src_x >> 2] >> (6 - 2 * (src_x & 3))) & 3;
> +dst[x >> 2] &= 0xff3f >> j2;
>  dst[x >> 2] |= b << (6 - j2);
>  }
>  if ((mask << j) & 0x80)
> @@ -176,11 +176,11 @@ static void png_put_interlaced_row(uint8_t *dst, int 
> width,
>  case 4:
>  src_x = 0;
>  for (x = 0; x < width; x++) {
> -int j2 = 4*(x&1);
> +int j2 = 4 * (x & 1);
>  j = (x & 7);
>  if ((dsp_mask << j) & 0x80) {
> -b = (src[src_x >> 1] >> (4 - 4*(src_x & 1))) & 15;
> -dst[x >> 1] &= 0xFF0F>>j2;
> +b = (src[src_x >> 1] >> (4 - 4 * (src_x & 1))) & 15;
> +dst[x >> 1] &= 0xff0f >> j2;
>  dst[x >> 1] |= b << (4 - j2);
>  }
>  if ((mask << j) & 0x80)
> @@ -191,15 +191,14 @@ static void png_put_interlaced_row(uint8_t *dst, int 
> width,
>  bpp = bits_per_pixel >> 3;
>  d   = dst;
>  s   = src;
> -for (x = 0; x < width; x++) {
> -j = x & 7;
> -if ((dsp_mask << j) & 0x80) {
> -memcpy(d, s, bpp);
> -}
> -d += bpp;
> -if ((mask << j) & 0x80)
> -s += bpp;
> -}
> +for (x = 0; x < width; x++) {
> +j = x & 7;
> +if ((dsp_mask << j) & 0x80)
> +memcpy(d, s, bpp);
> +d += bpp;
> +if ((mask << j) & 0x80)
> +s += bpp;
> +}
>  break;
>  }
>  }
> @@ -207,8 +206,7 @@ static void png_put_interlaced_row(uint8_t *dst, int 
> width,
>  void ff_add_png_paeth_prediction(uint8_t *dst, uint8_t *src, uint8_t *top,
>   int w, int bpp)
>  {
> -int i;
> -for (i = 0; i < w; i++) {
> +for (int i = 0; i < w; i++) {
>  int a, b, c, p, pa, pb, pc;
>  
>  a = dst[i - bpp];
> @@ -233,7 +231,7 @@ void ff_add_png_paeth_prediction(uint8_t *dst, uint8_t 
> *src, uint8_t *top,
>  }
>  
>  #define UNROLL1(bpp, op) 
>  \
> -{
>  \
> +do { 
>  \
>  r = dst[0];  
>  \
>  if (bpp >= 2)
>  \
>  g = dst[1];  
>  \
> @@ -253,21 +251,21 @@ void ff_add_png_paeth_prediction(uint8_t *dst, uint8_t 
> *src, uint8_t *top,
>  continue;
>  \
>  dst[i + 3] = a = op(a, src[i + 3], last[i + 3]); 
>  \
>  }
>  \
> -}
> -
> -#define UNROLL_FILTER(op)
>  \
> -if (bpp == 1) {  
>  \
> -UNROLL1(1, op)   
>  \
> -} else if (bpp == 2) {   
>  \
> -UNROLL1(2, op

[FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes

[Leo Izen]

Various parts of this file are restructured slightly for readability,
such as spacing in arithmetic operations, and putting `if (ret < 0)`
clauses on separate lines.

Signed-off-by: Leo Izen 
---
 libavcodec/pngdec.c | 241 +++-
 1 file changed, 124 insertions(+), 117 deletions(-)

diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index d812ffd348..93b067a9be 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -128,7 +128,7 @@ static const uint8_t png_pass_dsp_ymask[NB_PASSES] = {
 
 /* Mask to determine which pixels to overwrite while displaying */
 static const uint8_t png_pass_dsp_mask[NB_PASSES] = {
-0xff, 0x0f, 0xff, 0x33, 0xff, 0x55, 0xff
+0xff, 0x0f, 0xff, 0x33, 0xff, 0x55, 0xff,
 };
 
 /* NOTE: we try to construct a good looking image at each pass. width
@@ -152,7 +152,7 @@ static void png_put_interlaced_row(uint8_t *dst, int width,
 j = (x & 7);
 if ((dsp_mask << j) & 0x80) {
 b = (src[src_x >> 3] >> (7 - (src_x & 7))) & 1;
-dst[x >> 3] &= 0xFF7F>>j;
+dst[x >> 3] &= 0xff7f >> j;
 dst[x >> 3] |= b << (7 - j);
 }
 if ((mask << j) & 0x80)
@@ -165,8 +165,8 @@ static void png_put_interlaced_row(uint8_t *dst, int width,
 int j2 = 2 * (x & 3);
 j = (x & 7);
 if ((dsp_mask << j) & 0x80) {
-b = (src[src_x >> 2] >> (6 - 2*(src_x & 3))) & 3;
-dst[x >> 2] &= 0xFF3F>>j2;
+b = (src[src_x >> 2] >> (6 - 2 * (src_x & 3))) & 3;
+dst[x >> 2] &= 0xff3f >> j2;
 dst[x >> 2] |= b << (6 - j2);
 }
 if ((mask << j) & 0x80)
@@ -176,11 +176,11 @@ static void png_put_interlaced_row(uint8_t *dst, int 
width,
 case 4:
 src_x = 0;
 for (x = 0; x < width; x++) {
-int j2 = 4*(x&1);
+int j2 = 4 * (x & 1);
 j = (x & 7);
 if ((dsp_mask << j) & 0x80) {
-b = (src[src_x >> 1] >> (4 - 4*(src_x & 1))) & 15;
-dst[x >> 1] &= 0xFF0F>>j2;
+b = (src[src_x >> 1] >> (4 - 4 * (src_x & 1))) & 15;
+dst[x >> 1] &= 0xff0f >> j2;
 dst[x >> 1] |= b << (4 - j2);
 }
 if ((mask << j) & 0x80)
@@ -191,15 +191,14 @@ static void png_put_interlaced_row(uint8_t *dst, int 
width,
 bpp = bits_per_pixel >> 3;
 d   = dst;
 s   = src;
-for (x = 0; x < width; x++) {
-j = x & 7;
-if ((dsp_mask << j) & 0x80) {
-memcpy(d, s, bpp);
-}
-d += bpp;
-if ((mask << j) & 0x80)
-s += bpp;
-}
+for (x = 0; x < width; x++) {
+j = x & 7;
+if ((dsp_mask << j) & 0x80)
+memcpy(d, s, bpp);
+d += bpp;
+if ((mask << j) & 0x80)
+s += bpp;
+}
 break;
 }
 }
@@ -207,8 +206,7 @@ static void png_put_interlaced_row(uint8_t *dst, int width,
 void ff_add_png_paeth_prediction(uint8_t *dst, uint8_t *src, uint8_t *top,
  int w, int bpp)
 {
-int i;
-for (i = 0; i < w; i++) {
+for (int i = 0; i < w; i++) {
 int a, b, c, p, pa, pb, pc;
 
 a = dst[i - bpp];
@@ -233,7 +231,7 @@ void ff_add_png_paeth_prediction(uint8_t *dst, uint8_t 
*src, uint8_t *top,
 }
 
 #define UNROLL1(bpp, op)  \
-{ \
+do {  \
 r = dst[0];   \
 if (bpp >= 2) \
 g = dst[1];   \
@@ -253,21 +251,21 @@ void ff_add_png_paeth_prediction(uint8_t *dst, uint8_t 
*src, uint8_t *top,
 continue; \
 dst[i + 3] = a = op(a, src[i + 3], last[i + 3]);  \
 } \
-}
-
-#define UNROLL_FILTER(op) \
-if (bpp == 1) {   \
-UNROLL1(1, op)\
-} else if (bpp == 2) {\
-UNROLL1(2, op)\
-} else if (bpp == 3) {\
-UNROLL1(3, op)\
-} else if (bpp == 4) {