* [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes
@ 2023-10-24 11:13 Leo Izen
2023-10-26 11:04 ` Andreas Rheinhardt
2023-10-26 19:25 ` Michael Niedermayer
0 siblings, 2 replies; 3+ messages in thread
From: Leo Izen @ 2023-10-24 11:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: 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 <leo.izen@gmail.com>
---
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) { \
- UNROLL1(4, op) \
- } \
- for (; i < size; i++) { \
+ } while (0)
+
+#define UNROLL_FILTER(op) do { \
+ if (bpp == 1) \
+ UNROLL1(1, op); \
+ else if (bpp == 2) \
+ UNROLL1(2, op); \
+ else if (bpp == 3) \
+ UNROLL1(3, op); \
+ else if (bpp == 4) \
+ UNROLL1(4, op); \
+ \
+ for (; i < size; i++) \
dst[i] = op(dst[i - bpp], src[i], last[i]); \
- }
+} while (0)
/* NOTE: 'dst' can be equal to 'last' */
void ff_png_filter_row(PNGDSPContext *dsp, uint8_t *dst, int filter_type,
@@ -330,9 +328,8 @@ void ff_png_filter_row(PNGDSPContext *dsp, uint8_t *dst, int filter_type,
#define YUV2RGB(NAME, TYPE) \
static void deloco_ ## NAME(TYPE *dst, int size, int alpha) \
{ \
- int i; \
- for (i = 0; i < size - 2; i += 3 + alpha) { \
- int g = dst [i + 1]; \
+ for (int i = 0; i < size - 2; i += 3 + alpha) { \
+ int g = dst[i + 1]; \
dst[i + 0] += g; \
dst[i + 2] += g; \
} \
@@ -343,11 +340,10 @@ YUV2RGB(rgb16, uint16_t)
static int percent_missing(PNGDecContext *s)
{
- if (s->interlace_type) {
+ if (s->interlace_type)
return 100 - 100 * s->pass / (NB_PASSES - 1);
- } else {
+ else
return 100 - 100 * s->y / s->cur_h;
- }
}
/* process exactly one decompressed row */
@@ -367,30 +363,28 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride)
last_row, s->row_size, s->bpp);
/* loco lags by 1 row so that it doesn't interfere with top prediction */
if (s->filter_type == PNG_FILTER_TYPE_LOCO && s->y > 0) {
- if (s->bit_depth == 16) {
+ if (s->bit_depth == 16)
deloco_rgb16((uint16_t *)(ptr - dst_stride), s->row_size / 2,
s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
- } else {
+ else
deloco_rgb8(ptr - dst_stride, s->row_size,
s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
- }
}
s->y++;
if (s->y == s->cur_h) {
s->pic_state |= PNG_ALLIMAGE;
if (s->filter_type == PNG_FILTER_TYPE_LOCO) {
- if (s->bit_depth == 16) {
+ if (s->bit_depth == 16)
deloco_rgb16((uint16_t *)ptr, s->row_size / 2,
s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
- } else {
+ else
deloco_rgb8(ptr, s->row_size,
s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
- }
}
}
} else {
got_line = 0;
- for (;;) {
+ while (1) {
ptr = dst + dst_stride * (s->y + s->y_offset) + s->x_offset * s->bpp;
if ((ff_png_pass_ymask[s->pass] << (s->y & 7)) & 0x80) {
/* if we already read one row, it is time to stop to
@@ -403,17 +397,16 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride)
FFSWAP(unsigned int, s->last_row_size, s->tmp_row_size);
got_line = 1;
}
- if ((png_pass_dsp_ymask[s->pass] << (s->y & 7)) & 0x80) {
+ if ((png_pass_dsp_ymask[s->pass] << (s->y & 7)) & 0x80)
png_put_interlaced_row(ptr, s->cur_w, s->bits_per_pixel, s->pass,
s->color_type, s->last_row);
- }
s->y++;
if (s->y == s->cur_h) {
memset(s->last_row, 0, s->row_size);
- for (;;) {
+ while (1) {
if (s->pass == NB_PASSES - 1) {
s->pic_state |= PNG_ALLIMAGE;
- goto the_end;
+ return;
} else {
s->pass++;
s->y = 0;
@@ -428,7 +421,6 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride)
}
}
}
-the_end:;
}
}
@@ -448,18 +440,18 @@ static int png_decode_idat(PNGDecContext *s, GetByteContext *gb,
return AVERROR_EXTERNAL;
}
if (zstream->avail_out == 0) {
- if (!(s->pic_state & PNG_ALLIMAGE)) {
+ if (!(s->pic_state & PNG_ALLIMAGE))
png_handle_row(s, dst, dst_stride);
- }
zstream->avail_out = s->crow_size;
zstream->next_out = s->crow_buf;
}
if (ret == Z_STREAM_END && zstream->avail_in > 0) {
av_log(s->avctx, AV_LOG_WARNING,
"%d undecompressed bytes left in buffer\n", zstream->avail_in);
- return 0;
+ break;
}
}
+
return 0;
}
@@ -538,7 +530,7 @@ static int decode_text_chunk(PNGDecContext *s, GetByteContext *gb, int compresse
const char *keyword_end = memchr(keyword, 0, data_end - data);
char *kw_utf8 = NULL, *txt_utf8 = NULL;
const char *text;
- unsigned text_len;
+ size_t text_len;
AVBPrint bp;
if (!keyword_end)
@@ -551,7 +543,8 @@ static int decode_text_chunk(PNGDecContext *s, GetByteContext *gb, int compresse
method = *(data++);
if (method)
return AVERROR_INVALIDDATA;
- if ((ret = decode_zbuf(&bp, data, data_end, s->avctx)) < 0)
+ ret = decode_zbuf(&bp, data, data_end, s->avctx);
+ if (ret < 0)
return ret;
text = bp.str;
text_len = bp.len;
@@ -637,7 +630,7 @@ static int decode_phys_chunk(AVCodecContext *avctx, PNGDecContext *s,
avctx->sample_aspect_ratio.num = bytestream2_get_be32(gb);
avctx->sample_aspect_ratio.den = bytestream2_get_be32(gb);
if (avctx->sample_aspect_ratio.num < 0 || avctx->sample_aspect_ratio.den < 0)
- avctx->sample_aspect_ratio = (AVRational){ 0, 1 };
+ avctx->sample_aspect_ratio = av_make_q(0, 1);
bytestream2_skip(gb, 1); /* unit specifier */
return 0;
@@ -833,8 +826,8 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
return ret;
} else {
/* The picture output this time and the reference to retain coincide. */
- if ((ret = ff_thread_get_ext_buffer(avctx, &s->picture,
- AV_GET_BUFFER_FLAG_REF)) < 0)
+ ret = ff_thread_get_ext_buffer(avctx, &s->picture, AV_GET_BUFFER_FLAG_REF);
+ if (ret < 0)
return ret;
ret = av_frame_ref(p, s->picture.f);
if (ret < 0)
@@ -845,7 +838,8 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
p->flags |= AV_FRAME_FLAG_KEY;
p->flags |= AV_FRAME_FLAG_INTERLACED * !!s->interlace_type;
- if ((ret = populate_avctx_color_fields(avctx, p)) < 0)
+ ret = populate_avctx_color_fields(avctx, p);
+ if (ret < 0)
return ret;
ff_thread_finish_setup(avctx);
@@ -992,7 +986,8 @@ static int decode_iccp_chunk(PNGDecContext *s, GetByteContext *gb)
goto fail;
}
- if ((ret = decode_zbuf(&bp, gb->buffer, gb->buffer_end, s->avctx)) < 0)
+ ret = decode_zbuf(&bp, gb->buffer, gb->buffer_end, s->avctx);
+ if (ret < 0)
return ret;
av_freep(&s->iccp_data);
@@ -1050,17 +1045,17 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
for (j = 0; j < s->height; j++) {
i = s->width / 8;
for (k = 7; k >= 1; k--)
- if ((s->width&7) >= k)
- pd[8*i + k - 1] = (pd[i]>>8-k) & 1;
+ if ((s->width & 7) >= k)
+ pd[8*i + k - 1] = (pd[i] >> (8 - k)) & 1;
for (i--; i >= 0; i--) {
- pd[8*i + 7]= pd[i] & 1;
- pd[8*i + 6]= (pd[i]>>1) & 1;
- pd[8*i + 5]= (pd[i]>>2) & 1;
- pd[8*i + 4]= (pd[i]>>3) & 1;
- pd[8*i + 3]= (pd[i]>>4) & 1;
- pd[8*i + 2]= (pd[i]>>5) & 1;
- pd[8*i + 1]= (pd[i]>>6) & 1;
- pd[8*i + 0]= pd[i]>>7;
+ pd[8*i + 7] = pd[i] & 1;
+ pd[8*i + 6] = (pd[i] >> 1) & 1;
+ pd[8*i + 5] = (pd[i] >> 2) & 1;
+ pd[8*i + 4] = (pd[i] >> 3) & 1;
+ pd[8*i + 3] = (pd[i] >> 4) & 1;
+ pd[8*i + 2] = (pd[i] >> 5) & 1;
+ pd[8*i + 1] = (pd[i] >> 6) & 1;
+ pd[8*i + 0] = pd[i] >> 7;
}
pd += p->linesize[0];
}
@@ -1070,24 +1065,24 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
for (j = 0; j < s->height; j++) {
i = s->width / 4;
if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
- if ((s->width&3) >= 3) pd[4*i + 2]= (pd[i] >> 2) & 3;
- if ((s->width&3) >= 2) pd[4*i + 1]= (pd[i] >> 4) & 3;
- if ((s->width&3) >= 1) pd[4*i + 0]= pd[i] >> 6;
+ if ((s->width & 3) >= 3) pd[4*i + 2]= (pd[i] >> 2) & 3;
+ if ((s->width & 3) >= 2) pd[4*i + 1]= (pd[i] >> 4) & 3;
+ if ((s->width & 3) >= 1) pd[4*i + 0]= pd[i] >> 6;
for (i--; i >= 0; i--) {
- pd[4*i + 3]= pd[i] & 3;
- pd[4*i + 2]= (pd[i]>>2) & 3;
- pd[4*i + 1]= (pd[i]>>4) & 3;
- pd[4*i + 0]= pd[i]>>6;
+ pd[4*i + 3]= pd[i] & 3;
+ pd[4*i + 2]= (pd[i] >> 2) & 3;
+ pd[4*i + 1]= (pd[i] >> 4) & 3;
+ pd[4*i + 0]= pd[i] >> 6;
}
} else {
- if ((s->width&3) >= 3) pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55;
- if ((s->width&3) >= 2) pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55;
- if ((s->width&3) >= 1) pd[4*i + 0]= ( pd[i]>>6 )*0x55;
+ if ((s->width & 3) >= 3) pd[4*i + 2]= ((pd[i] >> 2) & 3) * 0x55;
+ if ((s->width & 3) >= 2) pd[4*i + 1]= ((pd[i] >> 4) & 3) * 0x55;
+ if ((s->width & 3) >= 1) pd[4*i + 0]= ( pd[i] >> 6 ) * 0x55;
for (i--; i >= 0; i--) {
- pd[4*i + 3]= ( pd[i] & 3)*0x55;
- pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55;
- pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55;
- pd[4*i + 0]= ( pd[i]>>6 )*0x55;
+ pd[4*i + 3]= ( pd[i] & 3) * 0x55;
+ pd[4*i + 2]= ((pd[i] >> 2) & 3) * 0x55;
+ pd[4*i + 1]= ((pd[i] >> 4) & 3) * 0x55;
+ pd[4*i + 0]= ( pd[i] >> 6 ) * 0x55;
}
}
pd += p->linesize[0];
@@ -1096,18 +1091,20 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
int i, j;
uint8_t *pd = p->data[0];
for (j = 0; j < s->height; j++) {
- i = s->width/2;
+ i = s->width / 2;
if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
- if (s->width&1) pd[2*i+0]= pd[i]>>4;
+ if (s->width & 1)
+ pd[2*i+0] = pd[i] >> 4;
for (i--; i >= 0; i--) {
- pd[2*i + 1] = pd[i] & 15;
+ pd[2*i + 1] = pd[i] & 0xff;
pd[2*i + 0] = pd[i] >> 4;
}
} else {
- if (s->width & 1) pd[2*i + 0]= (pd[i] >> 4) * 0x11;
- for (i--; i >= 0; i--) {
- pd[2*i + 1] = (pd[i] & 15) * 0x11;
+ if (s->width & 1)
pd[2*i + 0] = (pd[i] >> 4) * 0x11;
+ for (i--; i >= 0; i--) {
+ pd[2*i + 1] = (pd[i] & 0xff) * 0x11;
+ pd[2*i + 0] = (pd[i] >> 4) * 0x11;
}
}
pd += p->linesize[0];
@@ -1321,7 +1318,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
int decode_next_dat = 0;
int i, ret;
- for (;;) {
+ while (1) {
GetByteContext gb_chunk;
length = bytestream2_get_bytes_left(&s->gb);
@@ -1339,8 +1336,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
goto exit_loop;
}
av_log(avctx, AV_LOG_ERROR, "%d bytes left\n", length);
- if ( s->pic_state & PNG_ALLIMAGE
- && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL)
+ if (s->pic_state & PNG_ALLIMAGE && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL)
goto exit_loop;
ret = AVERROR_INVALIDDATA;
goto fail;
@@ -1405,7 +1401,8 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
case MKTAG('f', 'c', 'T', 'L'):
if (!CONFIG_APNG_DECODER || avctx->codec_id != AV_CODEC_ID_APNG)
continue;
- if ((ret = decode_fctl_chunk(avctx, s, &gb_chunk)) < 0)
+ ret = decode_fctl_chunk(avctx, s, &gb_chunk);
+ if (ret < 0)
goto fail;
decode_next_dat = 1;
break;
@@ -1421,14 +1418,19 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
case MKTAG('I', 'D', 'A', 'T'):
if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && !decode_next_dat)
continue;
- if ((ret = decode_idat_chunk(avctx, s, &gb_chunk, p)) < 0)
+ ret = decode_idat_chunk(avctx, s, &gb_chunk, p);
+ if (ret < 0)
goto fail;
break;
case MKTAG('P', 'L', 'T', 'E'):
- decode_plte_chunk(avctx, s, &gb_chunk);
+ ret = decode_plte_chunk(avctx, s, &gb_chunk);
+ if (ret < 0)
+ goto fail;
break;
case MKTAG('t', 'R', 'N', 'S'):
- decode_trns_chunk(avctx, s, &gb_chunk);
+ ret = decode_trns_chunk(avctx, s, &gb_chunk);
+ if (ret < 0)
+ goto fail;
break;
case MKTAG('t', 'E', 'X', 't'):
if (decode_text_chunk(s, &gb_chunk, 0) < 0)
@@ -1468,7 +1470,8 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
s->have_srgb = 1;
break;
case MKTAG('i', 'C', 'C', 'P'): {
- if ((ret = decode_iccp_chunk(s, &gb_chunk)) < 0)
+ ret = decode_iccp_chunk(s, &gb_chunk);
+ if (ret < 0)
goto fail;
break;
}
@@ -1487,22 +1490,24 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
break;
}
case MKTAG('s', 'B', 'I', 'T'):
- if ((ret = decode_sbit_chunk(avctx, s, &gb_chunk)) < 0)
+ ret = decode_sbit_chunk(avctx, s, &gb_chunk);
+ if (ret < 0)
goto fail;
break;
case MKTAG('g', 'A', 'M', 'A'): {
AVBPrint bp;
- char *gamma_str;
+ char *gamma_str = NULL;
s->gamma = bytestream2_get_be32(&gb_chunk);
av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
av_bprintf(&bp, "%i/%i", s->gamma, 100000);
ret = av_bprint_finalize(&bp, &gamma_str);
- if (ret < 0)
- return ret;
+ if (ret < 0) {
+ av_freep(&gamma_str);
+ goto fail;
+ }
av_dict_set(&s->frame_metadata, "gamma", gamma_str, AV_DICT_DONT_STRDUP_VAL);
-
break;
}
case MKTAG('I', 'E', 'N', 'D'):
@@ -1540,10 +1545,10 @@ exit_loop:
for (int x = s->width - 1; x >= 0; x--) {
const uint8_t idx = row[x];
- row[4*x+2] = s->palette[idx] & 0xFF;
- row[4*x+1] = (s->palette[idx] >> 8 ) & 0xFF;
- row[4*x+0] = (s->palette[idx] >> 16) & 0xFF;
- row[4*x+3] = s->palette[idx] >> 24;
+ row[4*x + 2] = s->palette[idx] & 0xFF;
+ row[4*x + 1] = (s->palette[idx] >> 8 ) & 0xFF;
+ row[4*x + 0] = (s->palette[idx] >> 16) & 0xFF;
+ row[4*x + 3] = s->palette[idx] >> 24;
}
}
}
@@ -1572,7 +1577,7 @@ exit_loop:
uint8_t *rowp = &row[3 * s->width - 1];
int tcolor = AV_RL24(s->transparent_color_be);
for (x = s->width; x > 0; --x) {
- *pixel-- = AV_RL24(rowp-2) == tcolor ? 0 : 0xff;
+ *pixel-- = AV_RL24(rowp - 2) == tcolor ? 0 : 0xff;
*pixel-- = *rowp--;
*pixel-- = *rowp--;
*pixel-- = *rowp--;
@@ -1583,11 +1588,10 @@ exit_loop:
uint8_t *pixel = &row[s->bpp * (x - 1)];
memmove(pixel, &row[raw_bpp * (x - 1)], raw_bpp);
- if (!memcmp(pixel, s->transparent_color_be, raw_bpp)) {
+ if (!memcmp(pixel, s->transparent_color_be, raw_bpp))
memset(&pixel[raw_bpp], 0, byte_depth);
- } else {
+ else
memset(&pixel[raw_bpp], 0xff, byte_depth);
- }
}
}
}
@@ -1689,7 +1693,8 @@ static int decode_frame_png(AVCodecContext *avctx, AVFrame *p,
if (ret != Z_OK)
return AVERROR_EXTERNAL;
- if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
+ ret = decode_frame_common(avctx, s, p, avpkt);
+ if (ret < 0)
goto the_end;
if (avctx->skip_frame == AVDISCARD_ALL) {
@@ -1729,20 +1734,22 @@ static int decode_frame_apng(AVCodecContext *avctx, AVFrame *p,
if (!avctx->extradata_size)
return AVERROR_INVALIDDATA;
- if ((ret = inflateReset(&s->zstream.zstream)) != Z_OK)
+ if (inflateReset(&s->zstream.zstream) != Z_OK)
return AVERROR_EXTERNAL;
bytestream2_init(&s->gb, avctx->extradata, avctx->extradata_size);
- if ((ret = decode_frame_common(avctx, s, NULL, avpkt)) < 0)
+ ret = decode_frame_common(avctx, s, NULL, avpkt);
+ if (ret < 0)
return ret;
}
/* reset state for a new frame */
- if ((ret = inflateReset(&s->zstream.zstream)) != Z_OK)
+ if (inflateReset(&s->zstream.zstream) != Z_OK)
return AVERROR_EXTERNAL;
s->y = 0;
s->pic_state = 0;
bytestream2_init(&s->gb, avpkt->data, avpkt->size);
- if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
+ ret = decode_frame_common(avctx, s, p, avpkt);
+ if (ret < 0)
return ret;
if (!(s->pic_state & PNG_ALLIMAGE))
--
2.42.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] 3+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes
2023-10-24 11:13 [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes Leo Izen
@ 2023-10-26 11:04 ` Andreas Rheinhardt
2023-10-26 19:25 ` Michael Niedermayer
1 sibling, 0 replies; 3+ messages in thread
From: Andreas Rheinhardt @ 2023-10-26 11:04 UTC (permalink / raw)
To: ffmpeg-devel
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 <leo.izen@gmail.com>
> ---
> 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) { \
> - UNROLL1(4, op) \
> - } \
> - for (; i < size; i++) { \
> + } while (0)
> +
> +#define UNROLL_FILTER(op) do { \
> + if (bpp == 1) \
> + UNROLL1(1, op); \
> + else if (bpp == 2) \
> + UNROLL1(2, op); \
> + else if (bpp == 3) \
> + UNROLL1(3, op); \
> + else if (bpp == 4) \
> + UNROLL1(4, op); \
> + \
> + for (; i < size; i++) \
> dst[i] = op(dst[i - bpp], src[i], last[i]); \
> - }
> +} while (0)
>
> /* NOTE: 'dst' can be equal to 'last' */
> void ff_png_filter_row(PNGDSPContext *dsp, uint8_t *dst, int filter_type,
> @@ -330,9 +328,8 @@ void ff_png_filter_row(PNGDSPContext *dsp, uint8_t *dst, int filter_type,
> #define YUV2RGB(NAME, TYPE) \
> static void deloco_ ## NAME(TYPE *dst, int size, int alpha) \
> { \
> - int i; \
> - for (i = 0; i < size - 2; i += 3 + alpha) { \
> - int g = dst [i + 1]; \
> + for (int i = 0; i < size - 2; i += 3 + alpha) { \
> + int g = dst[i + 1]; \
> dst[i + 0] += g; \
> dst[i + 2] += g; \
> } \
> @@ -343,11 +340,10 @@ YUV2RGB(rgb16, uint16_t)
>
> static int percent_missing(PNGDecContext *s)
> {
> - if (s->interlace_type) {
> + if (s->interlace_type)
> return 100 - 100 * s->pass / (NB_PASSES - 1);
> - } else {
> + else
> return 100 - 100 * s->y / s->cur_h;
> - }
> }
>
> /* process exactly one decompressed row */
> @@ -367,30 +363,28 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride)
> last_row, s->row_size, s->bpp);
> /* loco lags by 1 row so that it doesn't interfere with top prediction */
> if (s->filter_type == PNG_FILTER_TYPE_LOCO && s->y > 0) {
> - if (s->bit_depth == 16) {
> + if (s->bit_depth == 16)
> deloco_rgb16((uint16_t *)(ptr - dst_stride), s->row_size / 2,
> s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
> - } else {
> + else
> deloco_rgb8(ptr - dst_stride, s->row_size,
> s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
> - }
> }
> s->y++;
> if (s->y == s->cur_h) {
> s->pic_state |= PNG_ALLIMAGE;
> if (s->filter_type == PNG_FILTER_TYPE_LOCO) {
> - if (s->bit_depth == 16) {
> + if (s->bit_depth == 16)
> deloco_rgb16((uint16_t *)ptr, s->row_size / 2,
> s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
> - } else {
> + else
> deloco_rgb8(ptr, s->row_size,
> s->color_type == PNG_COLOR_TYPE_RGB_ALPHA);
> - }
> }
> }
> } else {
> got_line = 0;
> - for (;;) {
> + while (1) {
> ptr = dst + dst_stride * (s->y + s->y_offset) + s->x_offset * s->bpp;
> if ((ff_png_pass_ymask[s->pass] << (s->y & 7)) & 0x80) {
> /* if we already read one row, it is time to stop to
> @@ -403,17 +397,16 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride)
> FFSWAP(unsigned int, s->last_row_size, s->tmp_row_size);
> got_line = 1;
> }
> - if ((png_pass_dsp_ymask[s->pass] << (s->y & 7)) & 0x80) {
> + if ((png_pass_dsp_ymask[s->pass] << (s->y & 7)) & 0x80)
> png_put_interlaced_row(ptr, s->cur_w, s->bits_per_pixel, s->pass,
> s->color_type, s->last_row);
> - }
> s->y++;
> if (s->y == s->cur_h) {
> memset(s->last_row, 0, s->row_size);
> - for (;;) {
> + while (1) {
> if (s->pass == NB_PASSES - 1) {
> s->pic_state |= PNG_ALLIMAGE;
> - goto the_end;
> + return;
> } else {
> s->pass++;
> s->y = 0;
> @@ -428,7 +421,6 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride)
> }
> }
> }
> -the_end:;
> }
> }
>
> @@ -448,18 +440,18 @@ static int png_decode_idat(PNGDecContext *s, GetByteContext *gb,
> return AVERROR_EXTERNAL;
> }
> if (zstream->avail_out == 0) {
> - if (!(s->pic_state & PNG_ALLIMAGE)) {
> + if (!(s->pic_state & PNG_ALLIMAGE))
> png_handle_row(s, dst, dst_stride);
> - }
> zstream->avail_out = s->crow_size;
> zstream->next_out = s->crow_buf;
> }
> if (ret == Z_STREAM_END && zstream->avail_in > 0) {
> av_log(s->avctx, AV_LOG_WARNING,
> "%d undecompressed bytes left in buffer\n", zstream->avail_in);
> - return 0;
> + break;
> }
> }
> +
> return 0;
> }
>
> @@ -538,7 +530,7 @@ static int decode_text_chunk(PNGDecContext *s, GetByteContext *gb, int compresse
> const char *keyword_end = memchr(keyword, 0, data_end - data);
> char *kw_utf8 = NULL, *txt_utf8 = NULL;
> const char *text;
> - unsigned text_len;
> + size_t text_len;
> AVBPrint bp;
>
> if (!keyword_end)
> @@ -551,7 +543,8 @@ static int decode_text_chunk(PNGDecContext *s, GetByteContext *gb, int compresse
> method = *(data++);
> if (method)
> return AVERROR_INVALIDDATA;
> - if ((ret = decode_zbuf(&bp, data, data_end, s->avctx)) < 0)
> + ret = decode_zbuf(&bp, data, data_end, s->avctx);
> + if (ret < 0)
> return ret;
> text = bp.str;
> text_len = bp.len;
> @@ -637,7 +630,7 @@ static int decode_phys_chunk(AVCodecContext *avctx, PNGDecContext *s,
> avctx->sample_aspect_ratio.num = bytestream2_get_be32(gb);
> avctx->sample_aspect_ratio.den = bytestream2_get_be32(gb);
> if (avctx->sample_aspect_ratio.num < 0 || avctx->sample_aspect_ratio.den < 0)
> - avctx->sample_aspect_ratio = (AVRational){ 0, 1 };
> + avctx->sample_aspect_ratio = av_make_q(0, 1);
> bytestream2_skip(gb, 1); /* unit specifier */
>
> return 0;
> @@ -833,8 +826,8 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
> return ret;
> } else {
> /* The picture output this time and the reference to retain coincide. */
> - if ((ret = ff_thread_get_ext_buffer(avctx, &s->picture,
> - AV_GET_BUFFER_FLAG_REF)) < 0)
> + ret = ff_thread_get_ext_buffer(avctx, &s->picture, AV_GET_BUFFER_FLAG_REF);
> + if (ret < 0)
> return ret;
> ret = av_frame_ref(p, s->picture.f);
> if (ret < 0)
> @@ -845,7 +838,8 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
> p->flags |= AV_FRAME_FLAG_KEY;
> p->flags |= AV_FRAME_FLAG_INTERLACED * !!s->interlace_type;
>
> - if ((ret = populate_avctx_color_fields(avctx, p)) < 0)
> + ret = populate_avctx_color_fields(avctx, p);
> + if (ret < 0)
> return ret;
> ff_thread_finish_setup(avctx);
>
> @@ -992,7 +986,8 @@ static int decode_iccp_chunk(PNGDecContext *s, GetByteContext *gb)
> goto fail;
> }
>
> - if ((ret = decode_zbuf(&bp, gb->buffer, gb->buffer_end, s->avctx)) < 0)
> + ret = decode_zbuf(&bp, gb->buffer, gb->buffer_end, s->avctx);
> + if (ret < 0)
> return ret;
>
> av_freep(&s->iccp_data);
> @@ -1050,17 +1045,17 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
> for (j = 0; j < s->height; j++) {
> i = s->width / 8;
> for (k = 7; k >= 1; k--)
> - if ((s->width&7) >= k)
> - pd[8*i + k - 1] = (pd[i]>>8-k) & 1;
> + if ((s->width & 7) >= k)
> + pd[8*i + k - 1] = (pd[i] >> (8 - k)) & 1;
> for (i--; i >= 0; i--) {
> - pd[8*i + 7]= pd[i] & 1;
> - pd[8*i + 6]= (pd[i]>>1) & 1;
> - pd[8*i + 5]= (pd[i]>>2) & 1;
> - pd[8*i + 4]= (pd[i]>>3) & 1;
> - pd[8*i + 3]= (pd[i]>>4) & 1;
> - pd[8*i + 2]= (pd[i]>>5) & 1;
> - pd[8*i + 1]= (pd[i]>>6) & 1;
> - pd[8*i + 0]= pd[i]>>7;
> + pd[8*i + 7] = pd[i] & 1;
> + pd[8*i + 6] = (pd[i] >> 1) & 1;
> + pd[8*i + 5] = (pd[i] >> 2) & 1;
> + pd[8*i + 4] = (pd[i] >> 3) & 1;
> + pd[8*i + 3] = (pd[i] >> 4) & 1;
> + pd[8*i + 2] = (pd[i] >> 5) & 1;
> + pd[8*i + 1] = (pd[i] >> 6) & 1;
> + pd[8*i + 0] = pd[i] >> 7;
> }
> pd += p->linesize[0];
> }
> @@ -1070,24 +1065,24 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
> for (j = 0; j < s->height; j++) {
> i = s->width / 4;
> if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
> - if ((s->width&3) >= 3) pd[4*i + 2]= (pd[i] >> 2) & 3;
> - if ((s->width&3) >= 2) pd[4*i + 1]= (pd[i] >> 4) & 3;
> - if ((s->width&3) >= 1) pd[4*i + 0]= pd[i] >> 6;
> + if ((s->width & 3) >= 3) pd[4*i + 2]= (pd[i] >> 2) & 3;
> + if ((s->width & 3) >= 2) pd[4*i + 1]= (pd[i] >> 4) & 3;
> + if ((s->width & 3) >= 1) pd[4*i + 0]= pd[i] >> 6;
> for (i--; i >= 0; i--) {
> - pd[4*i + 3]= pd[i] & 3;
> - pd[4*i + 2]= (pd[i]>>2) & 3;
> - pd[4*i + 1]= (pd[i]>>4) & 3;
> - pd[4*i + 0]= pd[i]>>6;
> + pd[4*i + 3]= pd[i] & 3;
> + pd[4*i + 2]= (pd[i] >> 2) & 3;
> + pd[4*i + 1]= (pd[i] >> 4) & 3;
> + pd[4*i + 0]= pd[i] >> 6;
> }
> } else {
> - if ((s->width&3) >= 3) pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55;
> - if ((s->width&3) >= 2) pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55;
> - if ((s->width&3) >= 1) pd[4*i + 0]= ( pd[i]>>6 )*0x55;
> + if ((s->width & 3) >= 3) pd[4*i + 2]= ((pd[i] >> 2) & 3) * 0x55;
> + if ((s->width & 3) >= 2) pd[4*i + 1]= ((pd[i] >> 4) & 3) * 0x55;
> + if ((s->width & 3) >= 1) pd[4*i + 0]= ( pd[i] >> 6 ) * 0x55;
> for (i--; i >= 0; i--) {
> - pd[4*i + 3]= ( pd[i] & 3)*0x55;
> - pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55;
> - pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55;
> - pd[4*i + 0]= ( pd[i]>>6 )*0x55;
> + pd[4*i + 3]= ( pd[i] & 3) * 0x55;
> + pd[4*i + 2]= ((pd[i] >> 2) & 3) * 0x55;
> + pd[4*i + 1]= ((pd[i] >> 4) & 3) * 0x55;
> + pd[4*i + 0]= ( pd[i] >> 6 ) * 0x55;
> }
> }
> pd += p->linesize[0];
> @@ -1096,18 +1091,20 @@ static void handle_small_bpp(PNGDecContext *s, AVFrame *p)
> int i, j;
> uint8_t *pd = p->data[0];
> for (j = 0; j < s->height; j++) {
> - i = s->width/2;
> + i = s->width / 2;
> if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
> - if (s->width&1) pd[2*i+0]= pd[i]>>4;
> + if (s->width & 1)
> + pd[2*i+0] = pd[i] >> 4;
> for (i--; i >= 0; i--) {
> - pd[2*i + 1] = pd[i] & 15;
> + pd[2*i + 1] = pd[i] & 0xff;
> pd[2*i + 0] = pd[i] >> 4;
> }
> } else {
> - if (s->width & 1) pd[2*i + 0]= (pd[i] >> 4) * 0x11;
> - for (i--; i >= 0; i--) {
> - pd[2*i + 1] = (pd[i] & 15) * 0x11;
> + if (s->width & 1)
> pd[2*i + 0] = (pd[i] >> 4) * 0x11;
> + for (i--; i >= 0; i--) {
> + pd[2*i + 1] = (pd[i] & 0xff) * 0x11;
> + pd[2*i + 0] = (pd[i] >> 4) * 0x11;
> }
> }
> pd += p->linesize[0];
> @@ -1321,7 +1318,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> int decode_next_dat = 0;
> int i, ret;
>
> - for (;;) {
> + while (1) {
> GetByteContext gb_chunk;
>
> length = bytestream2_get_bytes_left(&s->gb);
> @@ -1339,8 +1336,7 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> goto exit_loop;
> }
> av_log(avctx, AV_LOG_ERROR, "%d bytes left\n", length);
> - if ( s->pic_state & PNG_ALLIMAGE
> - && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL)
> + if (s->pic_state & PNG_ALLIMAGE && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL)
> goto exit_loop;
> ret = AVERROR_INVALIDDATA;
> goto fail;
> @@ -1405,7 +1401,8 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> case MKTAG('f', 'c', 'T', 'L'):
> if (!CONFIG_APNG_DECODER || avctx->codec_id != AV_CODEC_ID_APNG)
> continue;
> - if ((ret = decode_fctl_chunk(avctx, s, &gb_chunk)) < 0)
> + ret = decode_fctl_chunk(avctx, s, &gb_chunk);
> + if (ret < 0)
> goto fail;
> decode_next_dat = 1;
> break;
> @@ -1421,14 +1418,19 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> case MKTAG('I', 'D', 'A', 'T'):
> if (CONFIG_APNG_DECODER && avctx->codec_id == AV_CODEC_ID_APNG && !decode_next_dat)
> continue;
> - if ((ret = decode_idat_chunk(avctx, s, &gb_chunk, p)) < 0)
> + ret = decode_idat_chunk(avctx, s, &gb_chunk, p);
> + if (ret < 0)
> goto fail;
> break;
> case MKTAG('P', 'L', 'T', 'E'):
> - decode_plte_chunk(avctx, s, &gb_chunk);
> + ret = decode_plte_chunk(avctx, s, &gb_chunk);
> + if (ret < 0)
> + goto fail;
This is a non-stylistic change and should definitely not be mixed with
stylistic stuff.
> break;
> case MKTAG('t', 'R', 'N', 'S'):
> - decode_trns_chunk(avctx, s, &gb_chunk);
> + ret = decode_trns_chunk(avctx, s, &gb_chunk);
> + if (ret < 0)
> + goto fail;
Same here.
> break;
> case MKTAG('t', 'E', 'X', 't'):
> if (decode_text_chunk(s, &gb_chunk, 0) < 0)
> @@ -1468,7 +1470,8 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> s->have_srgb = 1;
> break;
> case MKTAG('i', 'C', 'C', 'P'): {
> - if ((ret = decode_iccp_chunk(s, &gb_chunk)) < 0)
> + ret = decode_iccp_chunk(s, &gb_chunk);
> + if (ret < 0)
> goto fail;
> break;
> }
> @@ -1487,22 +1490,24 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
> break;
> }
> case MKTAG('s', 'B', 'I', 'T'):
> - if ((ret = decode_sbit_chunk(avctx, s, &gb_chunk)) < 0)
> + ret = decode_sbit_chunk(avctx, s, &gb_chunk);
> + if (ret < 0)
> goto fail;
> break;
> case MKTAG('g', 'A', 'M', 'A'): {
> AVBPrint bp;
> - char *gamma_str;
> + char *gamma_str = NULL;
> s->gamma = bytestream2_get_be32(&gb_chunk);
>
> av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> av_bprintf(&bp, "%i/%i", s->gamma, 100000);
> ret = av_bprint_finalize(&bp, &gamma_str);
> - if (ret < 0)
> - return ret;
> + if (ret < 0) {
> + av_freep(&gamma_str);
Unnecessary.
> + goto fail;
> + }
>
> av_dict_set(&s->frame_metadata, "gamma", gamma_str, AV_DICT_DONT_STRDUP_VAL);
> -
> break;
> }
> case MKTAG('I', 'E', 'N', 'D'):
> @@ -1540,10 +1545,10 @@ exit_loop:
> for (int x = s->width - 1; x >= 0; x--) {
> const uint8_t idx = row[x];
>
> - row[4*x+2] = s->palette[idx] & 0xFF;
> - row[4*x+1] = (s->palette[idx] >> 8 ) & 0xFF;
> - row[4*x+0] = (s->palette[idx] >> 16) & 0xFF;
> - row[4*x+3] = s->palette[idx] >> 24;
> + row[4*x + 2] = s->palette[idx] & 0xFF;
> + row[4*x + 1] = (s->palette[idx] >> 8 ) & 0xFF;
> + row[4*x + 0] = (s->palette[idx] >> 16) & 0xFF;
> + row[4*x + 3] = s->palette[idx] >> 24;
This (and the rest of the stylistic fixes) seem like unnecessary noise.
> }
> }
> }
> @@ -1572,7 +1577,7 @@ exit_loop:
> uint8_t *rowp = &row[3 * s->width - 1];
> int tcolor = AV_RL24(s->transparent_color_be);
> for (x = s->width; x > 0; --x) {
> - *pixel-- = AV_RL24(rowp-2) == tcolor ? 0 : 0xff;
> + *pixel-- = AV_RL24(rowp - 2) == tcolor ? 0 : 0xff;
> *pixel-- = *rowp--;
> *pixel-- = *rowp--;
> *pixel-- = *rowp--;
> @@ -1583,11 +1588,10 @@ exit_loop:
> uint8_t *pixel = &row[s->bpp * (x - 1)];
> memmove(pixel, &row[raw_bpp * (x - 1)], raw_bpp);
>
> - if (!memcmp(pixel, s->transparent_color_be, raw_bpp)) {
> + if (!memcmp(pixel, s->transparent_color_be, raw_bpp))
> memset(&pixel[raw_bpp], 0, byte_depth);
> - } else {
> + else
> memset(&pixel[raw_bpp], 0xff, byte_depth);
> - }
> }
> }
> }
> @@ -1689,7 +1693,8 @@ static int decode_frame_png(AVCodecContext *avctx, AVFrame *p,
> if (ret != Z_OK)
> return AVERROR_EXTERNAL;
>
> - if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
> + ret = decode_frame_common(avctx, s, p, avpkt);
> + if (ret < 0)
> goto the_end;
>
> if (avctx->skip_frame == AVDISCARD_ALL) {
> @@ -1729,20 +1734,22 @@ static int decode_frame_apng(AVCodecContext *avctx, AVFrame *p,
> if (!avctx->extradata_size)
> return AVERROR_INVALIDDATA;
>
> - if ((ret = inflateReset(&s->zstream.zstream)) != Z_OK)
> + if (inflateReset(&s->zstream.zstream) != Z_OK)
> return AVERROR_EXTERNAL;
> bytestream2_init(&s->gb, avctx->extradata, avctx->extradata_size);
> - if ((ret = decode_frame_common(avctx, s, NULL, avpkt)) < 0)
> + ret = decode_frame_common(avctx, s, NULL, avpkt);
> + if (ret < 0)
> return ret;
> }
>
> /* reset state for a new frame */
> - if ((ret = inflateReset(&s->zstream.zstream)) != Z_OK)
> + if (inflateReset(&s->zstream.zstream) != Z_OK)
> return AVERROR_EXTERNAL;
> s->y = 0;
> s->pic_state = 0;
> bytestream2_init(&s->gb, avpkt->data, avpkt->size);
> - if ((ret = decode_frame_common(avctx, s, p, avpkt)) < 0)
> + ret = decode_frame_common(avctx, s, p, avpkt);
> + if (ret < 0)
> return ret;
>
> if (!(s->pic_state & PNG_ALLIMAGE))
_______________________________________________
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] 3+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes
2023-10-24 11:13 [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes Leo Izen
2023-10-26 11:04 ` Andreas Rheinhardt
@ 2023-10-26 19:25 ` Michael Niedermayer
1 sibling, 0 replies; 3+ messages in thread
From: Michael Niedermayer @ 2023-10-26 19:25 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 958 bytes --]
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 <leo.izen@gmail.com>
> ---
> 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.
[-- 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] 3+ messages in thread
end of thread, other threads:[~2023-10-26 19:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-24 11:13 [FFmpeg-devel] [PATCH] avcodec/pngdec: various stylistic changes Leo Izen
2023-10-26 11:04 ` Andreas Rheinhardt
2023-10-26 19:25 ` 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