* [FFmpeg-devel] [PATCH 2/7] avcodec/iff: Avoid redundant frees
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
@ 2022-07-12 10:27 ` Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 3/7] avcodec/iff: Return early when possible Andreas Rheinhardt
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-07-12 10:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
This code is only called once during init, so none of the buffers
here have been allocated already.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/iff.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index 4abbed8dfb..f14644471b 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -246,8 +246,6 @@ static int extract_header(AVCodecContext *const avctx)
if (s->masking == MASK_HAS_MASK) {
if (s->bpp >= 8 && !s->ham) {
avctx->pix_fmt = AV_PIX_FMT_RGB32;
- av_freep(&s->mask_buf);
- av_freep(&s->mask_palbuf);
if (s->bpp > 16) {
av_log(avctx, AV_LOG_ERROR, "bpp %d too large for palette\n", s->bpp);
return AVERROR(ENOMEM);
@@ -256,10 +254,8 @@ static int extract_header(AVCodecContext *const avctx)
if (!s->mask_buf)
return AVERROR(ENOMEM);
s->mask_palbuf = av_malloc((2 << s->bpp) * sizeof(uint32_t) + AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->mask_palbuf) {
- av_freep(&s->mask_buf);
+ if (!s->mask_palbuf)
return AVERROR(ENOMEM);
- }
}
s->bpp++;
} else if (s->masking != MASK_NONE && s->masking != MASK_HAS_TRANSPARENT_COLOR) {
@@ -273,9 +269,6 @@ static int extract_header(AVCodecContext *const avctx)
if (s->video_size && s->planesize * s->bpp * avctx->height > s->video_size)
return AVERROR_INVALIDDATA;
- av_freep(&s->ham_buf);
- av_freep(&s->ham_palbuf);
-
if (s->ham) {
int i, count = FFMIN(palette_size / 3, 1 << s->ham);
int ham_count;
@@ -291,10 +284,8 @@ static int extract_header(AVCodecContext *const avctx)
ham_count = 8 * (1 << s->ham);
s->ham_palbuf = av_malloc(extra_space * (ham_count << !!(s->masking == MASK_HAS_MASK)) * sizeof (uint32_t) + AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->ham_palbuf) {
- av_freep(&s->ham_buf);
+ if (!s->ham_palbuf)
return AVERROR(ENOMEM);
- }
if (count) { // HAM with color palette attached
// prefill with black and palette and set HAM take direct value mask to zero
--
2.34.1
_______________________________________________
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] 8+ messages in thread* [FFmpeg-devel] [PATCH 3/7] avcodec/iff: Return early when possible
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 2/7] avcodec/iff: Avoid redundant frees Andreas Rheinhardt
@ 2022-07-12 10:27 ` Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 4/7] avcodec/iff: Pass extradata and extradata_size explicitly Andreas Rheinhardt
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-07-12 10:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
It allows to save one level of indentation.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/iff.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index f14644471b..13912e9133 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -223,7 +223,9 @@ static int extract_header(AVCodecContext *const avctx)
return AVERROR_INVALIDDATA;
}
- if (buf_size >= 41) {
+ if (buf_size < 41)
+ return 0;
+
s->compression = bytestream_get_byte(&buf);
s->bpp = bytestream_get_byte(&buf);
s->ham = bytestream_get_byte(&buf);
@@ -316,7 +318,6 @@ static int extract_header(AVCodecContext *const avctx)
s->ham_palbuf[(1 << s->bpp) + i] = s->ham_palbuf[i] | 0xFF000000;
}
}
- }
return 0;
}
@@ -1470,10 +1471,12 @@ static int parse_packet_header(AVCodecContext *const avctx,
IffContext *s = avctx->priv_data;
int i;
- if (avctx->codec_tag == MKTAG('A', 'N', 'I', 'M')) {
uint32_t chunk_id;
uint64_t data_size;
+ if (avctx->codec_tag != MKTAG('A', 'N', 'I', 'M'))
+ return 0;
+
bytestream2_skip(gb, 4);
while (bytestream2_get_bytes_left(gb) >= 1) {
chunk_id = bytestream2_get_le32(gb);
@@ -1517,7 +1520,6 @@ static int parse_packet_header(AVCodecContext *const avctx,
bytestream2_skip(gb, data_size + (data_size&1));
}
}
- }
return 0;
}
--
2.34.1
_______________________________________________
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] 8+ messages in thread* [FFmpeg-devel] [PATCH 4/7] avcodec/iff: Pass extradata and extradata_size explicitly
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 2/7] avcodec/iff: Avoid redundant frees Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 3/7] avcodec/iff: Return early when possible Andreas Rheinhardt
@ 2022-07-12 10:27 ` Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 5/7] avcodec/iff: Reindent after the previous commits Andreas Rheinhardt
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-07-12 10:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
This might be useful in case this decoder were changed to support
new extradata passed via side-data.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/iff.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index 13912e9133..453e910982 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -201,20 +201,20 @@ static int cmap_read_palette(AVCodecContext *avctx, uint32_t *pal)
* @param avctx the AVCodecContext where to extract extra context to
* @return >= 0 in case of success, a negative error code otherwise
*/
-static int extract_header(AVCodecContext *const avctx)
+static int extract_header(AVCodecContext *const avctx,
+ const uint8_t *const extradata, int extradata_size)
{
IffContext *s = avctx->priv_data;
- const uint8_t *buf;
+ const uint8_t *buf = extradata;
unsigned buf_size = 0;
int i, palette_size;
- if (avctx->extradata_size < 2) {
+ if (extradata_size < 2) {
av_log(avctx, AV_LOG_ERROR, "not enough extradata\n");
return AVERROR_INVALIDDATA;
}
- palette_size = avctx->extradata_size - AV_RB16(avctx->extradata);
+ palette_size = extradata_size - AV_RB16(extradata);
- buf = avctx->extradata;
buf_size = bytestream_get_be16(&buf);
if (buf_size <= 1 || palette_size < 0) {
av_log(avctx, AV_LOG_ERROR,
@@ -274,7 +274,7 @@ static int extract_header(AVCodecContext *const avctx)
if (s->ham) {
int i, count = FFMIN(palette_size / 3, 1 << s->ham);
int ham_count;
- const uint8_t *const palette = avctx->extradata + AV_RB16(avctx->extradata);
+ const uint8_t *const palette = extradata + AV_RB16(extradata);
int extra_space = 1;
if (avctx->codec_tag == MKTAG('P', 'B', 'M', ' ') && s->ham == 4)
@@ -389,7 +389,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
return AVERROR(ENOMEM);
}
- if ((err = extract_header(avctx)) < 0)
+ err = extract_header(avctx, avctx->extradata, avctx->extradata_size);
+ if (err < 0)
return err;
return 0;
--
2.34.1
_______________________________________________
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] 8+ messages in thread* [FFmpeg-devel] [PATCH 5/7] avcodec/iff: Reindent after the previous commits
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
` (2 preceding siblings ...)
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 4/7] avcodec/iff: Pass extradata and extradata_size explicitly Andreas Rheinhardt
@ 2022-07-12 10:27 ` Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 6/7] avcodec/iff: Remove transient objects from the context Andreas Rheinhardt
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-07-12 10:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/iff.c | 260 +++++++++++++++++++++++------------------------
1 file changed, 128 insertions(+), 132 deletions(-)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index 453e910982..00d0105be3 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -207,7 +207,7 @@ static int extract_header(AVCodecContext *const avctx,
IffContext *s = avctx->priv_data;
const uint8_t *buf = extradata;
unsigned buf_size = 0;
- int i, palette_size;
+ int palette_size;
if (extradata_size < 2) {
av_log(avctx, AV_LOG_ERROR, "not enough extradata\n");
@@ -215,109 +215,109 @@ static int extract_header(AVCodecContext *const avctx,
}
palette_size = extradata_size - AV_RB16(extradata);
- buf_size = bytestream_get_be16(&buf);
- if (buf_size <= 1 || palette_size < 0) {
- av_log(avctx, AV_LOG_ERROR,
- "Invalid palette size received: %u -> palette data offset: %d\n",
- buf_size, palette_size);
- return AVERROR_INVALIDDATA;
- }
+ buf_size = bytestream_get_be16(&buf);
+ if (buf_size <= 1 || palette_size < 0) {
+ av_log(avctx, AV_LOG_ERROR,
+ "Invalid palette size received: %u -> palette data offset: %d\n",
+ buf_size, palette_size);
+ return AVERROR_INVALIDDATA;
+ }
if (buf_size < 41)
return 0;
- s->compression = bytestream_get_byte(&buf);
- s->bpp = bytestream_get_byte(&buf);
- s->ham = bytestream_get_byte(&buf);
- s->flags = bytestream_get_byte(&buf);
- s->transparency = bytestream_get_be16(&buf);
- s->masking = bytestream_get_byte(&buf);
- for (i = 0; i < 16; i++)
- s->tvdc[i] = bytestream_get_be16(&buf);
-
- if (s->ham) {
- if (s->bpp > 8) {
- av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for HAM: %u\n", s->ham);
- return AVERROR_INVALIDDATA;
- } else if (s->ham != (s->bpp > 6 ? 6 : 4)) {
- av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for HAM: %u, BPP: %u\n", s->ham, s->bpp);
- return AVERROR_INVALIDDATA;
- }
+ s->compression = bytestream_get_byte(&buf);
+ s->bpp = bytestream_get_byte(&buf);
+ s->ham = bytestream_get_byte(&buf);
+ s->flags = bytestream_get_byte(&buf);
+ s->transparency = bytestream_get_be16(&buf);
+ s->masking = bytestream_get_byte(&buf);
+ for (int i = 0; i < 16; i++)
+ s->tvdc[i] = bytestream_get_be16(&buf);
+
+ if (s->ham) {
+ if (s->bpp > 8) {
+ av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for HAM: %u\n", s->ham);
+ return AVERROR_INVALIDDATA;
+ } else if (s->ham != (s->bpp > 6 ? 6 : 4)) {
+ av_log(avctx, AV_LOG_ERROR, "Invalid number of hold bits for HAM: %u, BPP: %u\n", s->ham, s->bpp);
+ return AVERROR_INVALIDDATA;
}
+ }
- if (s->masking == MASK_HAS_MASK) {
- if (s->bpp >= 8 && !s->ham) {
- avctx->pix_fmt = AV_PIX_FMT_RGB32;
- if (s->bpp > 16) {
- av_log(avctx, AV_LOG_ERROR, "bpp %d too large for palette\n", s->bpp);
- return AVERROR(ENOMEM);
- }
- s->mask_buf = av_malloc((s->planesize * 32) + AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->mask_buf)
- return AVERROR(ENOMEM);
- s->mask_palbuf = av_malloc((2 << s->bpp) * sizeof(uint32_t) + AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->mask_palbuf)
- return AVERROR(ENOMEM);
+ if (s->masking == MASK_HAS_MASK) {
+ if (s->bpp >= 8 && !s->ham) {
+ avctx->pix_fmt = AV_PIX_FMT_RGB32;
+ if (s->bpp > 16) {
+ av_log(avctx, AV_LOG_ERROR, "bpp %d too large for palette\n", s->bpp);
+ return AVERROR(ENOMEM);
}
- s->bpp++;
- } else if (s->masking != MASK_NONE && s->masking != MASK_HAS_TRANSPARENT_COLOR) {
- av_log(avctx, AV_LOG_ERROR, "Masking not supported\n");
- return AVERROR_PATCHWELCOME;
- }
- if (!s->bpp || s->bpp > 32) {
- av_log(avctx, AV_LOG_ERROR, "Invalid number of bitplanes: %u\n", s->bpp);
- return AVERROR_INVALIDDATA;
+ s->mask_buf = av_malloc((s->planesize * 32) + AV_INPUT_BUFFER_PADDING_SIZE);
+ if (!s->mask_buf)
+ return AVERROR(ENOMEM);
+ s->mask_palbuf = av_malloc((2 << s->bpp) * sizeof(uint32_t) + AV_INPUT_BUFFER_PADDING_SIZE);
+ if (!s->mask_palbuf)
+ return AVERROR(ENOMEM);
}
- if (s->video_size && s->planesize * s->bpp * avctx->height > s->video_size)
- return AVERROR_INVALIDDATA;
+ s->bpp++;
+ } else if (s->masking != MASK_NONE && s->masking != MASK_HAS_TRANSPARENT_COLOR) {
+ av_log(avctx, AV_LOG_ERROR, "Masking not supported\n");
+ return AVERROR_PATCHWELCOME;
+ }
+ if (!s->bpp || s->bpp > 32) {
+ av_log(avctx, AV_LOG_ERROR, "Invalid number of bitplanes: %u\n", s->bpp);
+ return AVERROR_INVALIDDATA;
+ }
+ if (s->video_size && s->planesize * s->bpp * avctx->height > s->video_size)
+ return AVERROR_INVALIDDATA;
- if (s->ham) {
- int i, count = FFMIN(palette_size / 3, 1 << s->ham);
- int ham_count;
- const uint8_t *const palette = extradata + AV_RB16(extradata);
- int extra_space = 1;
+ if (s->ham) {
+ int count = FFMIN(palette_size / 3, 1 << s->ham);
+ int ham_count;
+ const uint8_t *const palette = extradata + AV_RB16(extradata);
+ int extra_space = 1;
- if (avctx->codec_tag == MKTAG('P', 'B', 'M', ' ') && s->ham == 4)
- extra_space = 4;
+ if (avctx->codec_tag == MKTAG('P', 'B', 'M', ' ') && s->ham == 4)
+ extra_space = 4;
- s->ham_buf = av_malloc((s->planesize * 8) + AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->ham_buf)
- return AVERROR(ENOMEM);
+ s->ham_buf = av_malloc((s->planesize * 8) + AV_INPUT_BUFFER_PADDING_SIZE);
+ if (!s->ham_buf)
+ return AVERROR(ENOMEM);
- ham_count = 8 * (1 << s->ham);
- s->ham_palbuf = av_malloc(extra_space * (ham_count << !!(s->masking == MASK_HAS_MASK)) * sizeof (uint32_t) + AV_INPUT_BUFFER_PADDING_SIZE);
- if (!s->ham_palbuf)
- return AVERROR(ENOMEM);
+ ham_count = 8 * (1 << s->ham);
+ s->ham_palbuf = av_malloc(extra_space * (ham_count << !!(s->masking == MASK_HAS_MASK)) * sizeof (uint32_t) + AV_INPUT_BUFFER_PADDING_SIZE);
+ if (!s->ham_palbuf)
+ return AVERROR(ENOMEM);
- if (count) { // HAM with color palette attached
- // prefill with black and palette and set HAM take direct value mask to zero
- memset(s->ham_palbuf, 0, (1 << s->ham) * 2 * sizeof (uint32_t));
- for (i=0; i < count; i++) {
- s->ham_palbuf[i*2+1] = 0xFF000000 | AV_RL24(palette + i*3);
- }
- count = 1 << s->ham;
- } else { // HAM with grayscale color palette
- count = 1 << s->ham;
- for (i=0; i < count; i++) {
- s->ham_palbuf[i*2] = 0xFF000000; // take direct color value from palette
- s->ham_palbuf[i*2+1] = 0xFF000000 | av_le2ne32(gray2rgb((i * 255) >> s->ham));
- }
- }
- for (i=0; i < count; i++) {
- uint32_t tmp = i << (8 - s->ham);
- tmp |= tmp >> s->ham;
- s->ham_palbuf[(i+count)*2] = 0xFF00FFFF; // just modify blue color component
- s->ham_palbuf[(i+count*2)*2] = 0xFFFFFF00; // just modify red color component
- s->ham_palbuf[(i+count*3)*2] = 0xFFFF00FF; // just modify green color component
- s->ham_palbuf[(i+count)*2+1] = 0xFF000000 | tmp << 16;
- s->ham_palbuf[(i+count*2)*2+1] = 0xFF000000 | tmp;
- s->ham_palbuf[(i+count*3)*2+1] = 0xFF000000 | tmp << 8;
+ if (count) { // HAM with color palette attached
+ // prefill with black and palette and set HAM take direct value mask to zero
+ memset(s->ham_palbuf, 0, (1 << s->ham) * 2 * sizeof (uint32_t));
+ for (int i = 0; i < count; i++) {
+ s->ham_palbuf[i*2+1] = 0xFF000000 | AV_RL24(palette + i*3);
}
- if (s->masking == MASK_HAS_MASK) {
- for (i = 0; i < ham_count; i++)
- s->ham_palbuf[(1 << s->bpp) + i] = s->ham_palbuf[i] | 0xFF000000;
+ count = 1 << s->ham;
+ } else { // HAM with grayscale color palette
+ count = 1 << s->ham;
+ for (int i = 0; i < count; i++) {
+ s->ham_palbuf[i*2] = 0xFF000000; // take direct color value from palette
+ s->ham_palbuf[i*2+1] = 0xFF000000 | av_le2ne32(gray2rgb((i * 255) >> s->ham));
}
}
+ for (int i = 0; i < count; i++) {
+ uint32_t tmp = i << (8 - s->ham);
+ tmp |= tmp >> s->ham;
+ s->ham_palbuf[(i+count)*2] = 0xFF00FFFF; // just modify blue color component
+ s->ham_palbuf[(i+count*2)*2] = 0xFFFFFF00; // just modify red color component
+ s->ham_palbuf[(i+count*3)*2] = 0xFFFF00FF; // just modify green color component
+ s->ham_palbuf[(i+count)*2+1] = 0xFF000000 | tmp << 16;
+ s->ham_palbuf[(i+count*2)*2+1] = 0xFF000000 | tmp;
+ s->ham_palbuf[(i+count*3)*2+1] = 0xFF000000 | tmp << 8;
+ }
+ if (s->masking == MASK_HAS_MASK) {
+ for (int i = 0; i < ham_count; i++)
+ s->ham_palbuf[(1 << s->bpp) + i] = s->ham_palbuf[i] | 0xFF000000;
+ }
+ }
return 0;
}
@@ -1470,57 +1470,53 @@ static int parse_packet_header(AVCodecContext *const avctx,
GetByteContext *gb)
{
IffContext *s = avctx->priv_data;
- int i;
-
- uint32_t chunk_id;
- uint64_t data_size;
if (avctx->codec_tag != MKTAG('A', 'N', 'I', 'M'))
return 0;
- bytestream2_skip(gb, 4);
- while (bytestream2_get_bytes_left(gb) >= 1) {
- chunk_id = bytestream2_get_le32(gb);
- data_size = bytestream2_get_be32(gb);
-
- if (chunk_id == MKTAG('B', 'M', 'H', 'D')) {
- bytestream2_skip(gb, data_size + (data_size & 1));
- } else if (chunk_id == MKTAG('A', 'N', 'H', 'D')) {
- unsigned extra;
- if (data_size < 40)
- return AVERROR_INVALIDDATA;
-
- s->compression = (bytestream2_get_byte(gb) << 8) | (s->compression & 0xFF);
- bytestream2_skip(gb, 19);
- extra = bytestream2_get_be32(gb);
- s->is_short = !(extra & 1);
- s->is_brush = extra == 2;
- s->is_interlaced = !!(extra & 0x40);
- data_size -= 24;
- bytestream2_skip(gb, data_size + (data_size & 1));
- } else if (chunk_id == MKTAG('D', 'L', 'T', 'A') ||
- chunk_id == MKTAG('B', 'O', 'D', 'Y')) {
- if (chunk_id == MKTAG('B','O','D','Y'))
- s->compression &= 0xFF;
- break;
- } else if (chunk_id == MKTAG('C', 'M', 'A', 'P')) {
- int count = data_size / 3;
- uint32_t *pal = s->pal;
-
- if (count > 256)
- return AVERROR_INVALIDDATA;
- if (s->ham) {
- for (i = 0; i < count; i++)
- pal[i] = 0xFF000000 | bytestream2_get_le24(gb);
- } else {
- for (i = 0; i < count; i++)
- pal[i] = 0xFF000000 | bytestream2_get_be24(gb);
- }
- bytestream2_skip(gb, data_size & 1);
+ bytestream2_skip(gb, 4);
+ while (bytestream2_get_bytes_left(gb) >= 1) {
+ uint32_t chunk_id = bytestream2_get_le32(gb);
+ uint64_t data_size = bytestream2_get_be32(gb);
+
+ if (chunk_id == MKTAG('B', 'M', 'H', 'D')) {
+ bytestream2_skip(gb, data_size + (data_size & 1));
+ } else if (chunk_id == MKTAG('A', 'N', 'H', 'D')) {
+ unsigned extra;
+ if (data_size < 40)
+ return AVERROR_INVALIDDATA;
+
+ s->compression = (bytestream2_get_byte(gb) << 8) | (s->compression & 0xFF);
+ bytestream2_skip(gb, 19);
+ extra = bytestream2_get_be32(gb);
+ s->is_short = !(extra & 1);
+ s->is_brush = extra == 2;
+ s->is_interlaced = !!(extra & 0x40);
+ data_size -= 24;
+ bytestream2_skip(gb, data_size + (data_size & 1));
+ } else if (chunk_id == MKTAG('D', 'L', 'T', 'A') ||
+ chunk_id == MKTAG('B', 'O', 'D', 'Y')) {
+ if (chunk_id == MKTAG('B','O','D','Y'))
+ s->compression &= 0xFF;
+ break;
+ } else if (chunk_id == MKTAG('C', 'M', 'A', 'P')) {
+ int count = data_size / 3;
+ uint32_t *pal = s->pal;
+
+ if (count > 256)
+ return AVERROR_INVALIDDATA;
+ if (s->ham) {
+ for (int i = 0; i < count; i++)
+ pal[i] = 0xFF000000 | bytestream2_get_le24(gb);
} else {
- bytestream2_skip(gb, data_size + (data_size&1));
+ for (int i = 0; i < count; i++)
+ pal[i] = 0xFF000000 | bytestream2_get_be24(gb);
}
+ bytestream2_skip(gb, data_size & 1);
+ } else {
+ bytestream2_skip(gb, data_size + (data_size&1));
}
+ }
return 0;
}
--
2.34.1
_______________________________________________
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] 8+ messages in thread* [FFmpeg-devel] [PATCH 6/7] avcodec/iff: Remove transient objects from the context
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
` (3 preceding siblings ...)
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 5/7] avcodec/iff: Reindent after the previous commits Andreas Rheinhardt
@ 2022-07-12 10:27 ` Andreas Rheinhardt
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 7/7] avcodec/iff: Use unsigned to avoid compiler warning Andreas Rheinhardt
2022-09-15 0:52 ` [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-07-12 10:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
This avoids keeping invalid pointers in the context.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/iff.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index 00d0105be3..ad96bd9191 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -45,7 +45,6 @@ typedef enum {
} mask_type;
typedef struct IffContext {
- AVFrame *frame;
int planesize;
uint8_t * planebuf;
uint8_t * ham_buf; ///< temporary buffer for planar to chunky conversation
@@ -63,7 +62,6 @@ typedef struct IffContext {
unsigned masking; ///< TODO: masking method used
int init; // 1 if buffer and palette data already initialized, 0 otherwise
int16_t tvdc[16]; ///< TVDC lookup table
- GetByteContext gb;
uint8_t *video[2];
unsigned video_size;
uint32_t *pal;
@@ -1529,7 +1527,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
int buf_size = avpkt->size;
const uint8_t *buf_end = buf + buf_size;
int y, plane, res;
- GetByteContext *gb = &s->gb;
+ GetByteContext gb0, *const gb = &gb0;
const AVPixFmtDescriptor *desc;
bytestream2_init(gb, avpkt->data, avpkt->size);
@@ -1539,7 +1537,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
if ((res = ff_get_buffer(avctx, frame, 0)) < 0)
return res;
- s->frame = frame;
buf += bytestream2_tell(gb);
buf_size -= bytestream2_tell(gb);
@@ -1558,7 +1555,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
if (s->compression <= 0xff && (avctx->codec_tag == MKTAG('A', 'N', 'I', 'M'))) {
if (avctx->pix_fmt == AV_PIX_FMT_PAL8)
- memcpy(s->pal, s->frame->data[1], 256 * 4);
+ memcpy(s->pal, frame->data[1], 256 * 4);
}
switch (s->compression) {
--
2.34.1
_______________________________________________
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] 8+ messages in thread* [FFmpeg-devel] [PATCH 7/7] avcodec/iff: Use unsigned to avoid compiler warning
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
` (4 preceding siblings ...)
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 6/7] avcodec/iff: Remove transient objects from the context Andreas Rheinhardt
@ 2022-07-12 10:27 ` Andreas Rheinhardt
2022-09-15 0:52 ` [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-07-12 10:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
GCC 12 apparently believes that negative palette sizes are
possible (they are not, as this has already been checked during
init) and therefore emits a -Wstringop-overflow= for the memcpy.
Using unsigned avoids this.
(To be honest, there might be a compiler bug involved.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/iff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libavcodec/iff.c b/libavcodec/iff.c
index ad96bd9191..0bc2e3ca21 100644
--- a/libavcodec/iff.c
+++ b/libavcodec/iff.c
@@ -152,9 +152,10 @@ static av_always_inline uint32_t gray2rgb(const uint32_t x) {
static int cmap_read_palette(AVCodecContext *avctx, uint32_t *pal)
{
IffContext *s = avctx->priv_data;
- int count, i;
+ unsigned count, i;
const uint8_t *const palette = avctx->extradata + AV_RB16(avctx->extradata);
- int palette_size = avctx->extradata_size - AV_RB16(avctx->extradata);
+ /* extract_header() already checked that the RHS is >= 0. */
+ unsigned palette_size = avctx->extradata_size - AV_RB16(avctx->extradata);
if (avctx->bits_per_coded_sample > 8) {
av_log(avctx, AV_LOG_ERROR, "bits_per_coded_sample > 8 not supported\n");
--
2.34.1
_______________________________________________
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] 8+ messages in thread* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part
2022-07-12 10:26 [FFmpeg-devel] [PATCH 1/7] avcodec/iff: Split extract_header into extradata and packet part Andreas Rheinhardt
` (5 preceding siblings ...)
2022-07-12 10:27 ` [FFmpeg-devel] [PATCH 7/7] avcodec/iff: Use unsigned to avoid compiler warning Andreas Rheinhardt
@ 2022-09-15 0:52 ` Andreas Rheinhardt
6 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2022-09-15 0:52 UTC (permalink / raw)
To: ffmpeg-devel
Andreas Rheinhardt:
> 183132872a1d8bc8a32e7fd8f994fa2f1b2d6bfc made the iff demuxer
> output extradata and made the decoder parse said extradata.
> To make this extradata extensible, it came with its own internal
> length field (containing the offset of the palette at the end
> of the extradata). Furthermore, in order to support mid-stream
> extradata changes, the packets returned by the demuxer also have
> such a length field (containing the offset of the actual packet
> data). Therefore the packet parsing the extradata accepted its
> input from both AVPackets as well as from ordinary extradata.
>
> Yet the demuxer never made use of this "feature": The packet's
> length field always indicated that the packet data starts
> immediately after the length field.
>
> Later, commit cb928fc448f9566e6f6c28d53fa4c2388e732a2b stopped
> appending the length field to the packets' data; of course,
> it also stopped searching for extradata in this data.
>
> Instead it added code to parse the packet's header to the function
> that parses extradata. This made this function consist of two disjoint
> parts, one of which is only reachable if this function is called
> from init (when parsing extradata) and one of which is reachable
> when parsing packet headers.
>
> Therefore this commit splits this function into two.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Btw: Both of these commits broke API/ABI.
>
> libavcodec/iff.c | 116 +++++++++++++++++++++++++----------------------
> 1 file changed, 61 insertions(+), 55 deletions(-)
>
> diff --git a/libavcodec/iff.c b/libavcodec/iff.c
> index 74ebddc621..4abbed8dfb 100644
> --- a/libavcodec/iff.c
> +++ b/libavcodec/iff.c
> @@ -199,11 +199,9 @@ static int cmap_read_palette(AVCodecContext *avctx, uint32_t *pal)
> * decoder structures.
> *
> * @param avctx the AVCodecContext where to extract extra context to
> - * @param avpkt the AVPacket to extract extra context from or NULL to use avctx
> * @return >= 0 in case of success, a negative error code otherwise
> */
> -static int extract_header(AVCodecContext *const avctx,
> - const AVPacket *const avpkt)
> +static int extract_header(AVCodecContext *const avctx)
> {
> IffContext *s = avctx->priv_data;
> const uint8_t *buf;
> @@ -216,55 +214,6 @@ static int extract_header(AVCodecContext *const avctx,
> }
> palette_size = avctx->extradata_size - AV_RB16(avctx->extradata);
>
> - if (avpkt && avctx->codec_tag == MKTAG('A', 'N', 'I', 'M')) {
> - uint32_t chunk_id;
> - uint64_t data_size;
> - GetByteContext *gb = &s->gb;
> -
> - bytestream2_skip(gb, 4);
> - while (bytestream2_get_bytes_left(gb) >= 1) {
> - chunk_id = bytestream2_get_le32(gb);
> - data_size = bytestream2_get_be32(gb);
> -
> - if (chunk_id == MKTAG('B', 'M', 'H', 'D')) {
> - bytestream2_skip(gb, data_size + (data_size & 1));
> - } else if (chunk_id == MKTAG('A', 'N', 'H', 'D')) {
> - unsigned extra;
> - if (data_size < 40)
> - return AVERROR_INVALIDDATA;
> -
> - s->compression = (bytestream2_get_byte(gb) << 8) | (s->compression & 0xFF);
> - bytestream2_skip(gb, 19);
> - extra = bytestream2_get_be32(gb);
> - s->is_short = !(extra & 1);
> - s->is_brush = extra == 2;
> - s->is_interlaced = !!(extra & 0x40);
> - data_size -= 24;
> - bytestream2_skip(gb, data_size + (data_size & 1));
> - } else if (chunk_id == MKTAG('D', 'L', 'T', 'A') ||
> - chunk_id == MKTAG('B', 'O', 'D', 'Y')) {
> - if (chunk_id == MKTAG('B','O','D','Y'))
> - s->compression &= 0xFF;
> - break;
> - } else if (chunk_id == MKTAG('C', 'M', 'A', 'P')) {
> - int count = data_size / 3;
> - uint32_t *pal = s->pal;
> -
> - if (count > 256)
> - return AVERROR_INVALIDDATA;
> - if (s->ham) {
> - for (i = 0; i < count; i++)
> - pal[i] = 0xFF000000 | bytestream2_get_le24(gb);
> - } else {
> - for (i = 0; i < count; i++)
> - pal[i] = 0xFF000000 | bytestream2_get_be24(gb);
> - }
> - bytestream2_skip(gb, data_size & 1);
> - } else {
> - bytestream2_skip(gb, data_size + (data_size&1));
> - }
> - }
> - } else if (!avpkt) {
> buf = avctx->extradata;
> buf_size = bytestream_get_be16(&buf);
> if (buf_size <= 1 || palette_size < 0) {
> @@ -273,7 +222,6 @@ static int extract_header(AVCodecContext *const avctx,
> buf_size, palette_size);
> return AVERROR_INVALIDDATA;
> }
> - }
>
> if (buf_size >= 41) {
> s->compression = bytestream_get_byte(&buf);
> @@ -449,7 +397,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
> return AVERROR(ENOMEM);
> }
>
> - if ((err = extract_header(avctx, NULL)) < 0)
> + if ((err = extract_header(avctx)) < 0)
> return err;
>
> return 0;
> @@ -1525,6 +1473,64 @@ static int unsupported(AVCodecContext *avctx)
> return AVERROR_INVALIDDATA;
> }
>
> +static int parse_packet_header(AVCodecContext *const avctx,
> + GetByteContext *gb)
> +{
> + IffContext *s = avctx->priv_data;
> + int i;
> +
> + if (avctx->codec_tag == MKTAG('A', 'N', 'I', 'M')) {
> + uint32_t chunk_id;
> + uint64_t data_size;
> +
> + bytestream2_skip(gb, 4);
> + while (bytestream2_get_bytes_left(gb) >= 1) {
> + chunk_id = bytestream2_get_le32(gb);
> + data_size = bytestream2_get_be32(gb);
> +
> + if (chunk_id == MKTAG('B', 'M', 'H', 'D')) {
> + bytestream2_skip(gb, data_size + (data_size & 1));
> + } else if (chunk_id == MKTAG('A', 'N', 'H', 'D')) {
> + unsigned extra;
> + if (data_size < 40)
> + return AVERROR_INVALIDDATA;
> +
> + s->compression = (bytestream2_get_byte(gb) << 8) | (s->compression & 0xFF);
> + bytestream2_skip(gb, 19);
> + extra = bytestream2_get_be32(gb);
> + s->is_short = !(extra & 1);
> + s->is_brush = extra == 2;
> + s->is_interlaced = !!(extra & 0x40);
> + data_size -= 24;
> + bytestream2_skip(gb, data_size + (data_size & 1));
> + } else if (chunk_id == MKTAG('D', 'L', 'T', 'A') ||
> + chunk_id == MKTAG('B', 'O', 'D', 'Y')) {
> + if (chunk_id == MKTAG('B','O','D','Y'))
> + s->compression &= 0xFF;
> + break;
> + } else if (chunk_id == MKTAG('C', 'M', 'A', 'P')) {
> + int count = data_size / 3;
> + uint32_t *pal = s->pal;
> +
> + if (count > 256)
> + return AVERROR_INVALIDDATA;
> + if (s->ham) {
> + for (i = 0; i < count; i++)
> + pal[i] = 0xFF000000 | bytestream2_get_le24(gb);
> + } else {
> + for (i = 0; i < count; i++)
> + pal[i] = 0xFF000000 | bytestream2_get_be24(gb);
> + }
> + bytestream2_skip(gb, data_size & 1);
> + } else {
> + bytestream2_skip(gb, data_size + (data_size&1));
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
> int *got_frame, AVPacket *avpkt)
> {
> @@ -1538,7 +1544,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
>
> bytestream2_init(gb, avpkt->data, avpkt->size);
>
> - if ((res = extract_header(avctx, avpkt)) < 0)
> + if ((res = parse_packet_header(avctx, gb)) < 0)
> return res;
>
> if ((res = ff_get_buffer(avctx, frame, 0)) < 0)
Will apply this patchset tonight unless there are objections.
- Andreas
_______________________________________________
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] 8+ messages in thread