* [FFmpeg-devel] [PATCH 2/7] avcodec/sgidec: Support negative linesizes
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
@ 2022-09-30 17:05 ` Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 3/7] avcodec/sgidec: Avoid redundant private context Andreas Rheinhardt
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-09-30 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
The earlier code used "p->data[0] + p->linesize[0] * s->height" with
the latter being unsigned, which gives the wrong value for negative
linesizes. There is also a not so obvious problem with this:
In case of negative linesizes, the last line is the start of
the allocated buffer, so using the line after the last line
would involve undefined pointer arithmetic. So don't do it.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/sgidec.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/libavcodec/sgidec.c b/libavcodec/sgidec.c
index dd3dc46b48..a449859bf8 100644
--- a/libavcodec/sgidec.c
+++ b/libavcodec/sgidec.c
@@ -28,7 +28,7 @@
typedef struct SgiState {
AVCodecContext *avctx;
unsigned int width;
- unsigned int height;
+ int height;
unsigned int depth;
unsigned int bytes_per_channel;
int linesize;
@@ -127,12 +127,11 @@ static int expand_rle_row16(SgiState *s, uint16_t *out_buf,
* @param s the current image state
* @return 0 if no error, else return error code.
*/
-static int read_rle_sgi(uint8_t *out_buf, SgiState *s)
+static int read_rle_sgi(uint8_t *last_line, SgiState *s)
{
uint8_t *dest_row;
unsigned int len = s->height * s->depth * 4;
GetByteContext g_table = s->g;
- unsigned int y, z;
unsigned int start_offset;
int linesize, ret;
@@ -141,11 +140,10 @@ static int read_rle_sgi(uint8_t *out_buf, SgiState *s)
return AVERROR_INVALIDDATA;
}
- for (z = 0; z < s->depth; z++) {
- dest_row = out_buf;
- for (y = 0; y < s->height; y++) {
+ for (unsigned z = 0; z < s->depth; z++) {
+ dest_row = last_line;
+ for (int remaining_lines = s->height;;) {
linesize = s->width * s->depth;
- dest_row -= s->linesize;
start_offset = bytestream2_get_be32(&g_table);
bytestream2_seek(&s->g, start_offset, SEEK_SET);
if (s->bytes_per_channel == 1)
@@ -154,6 +152,9 @@ static int read_rle_sgi(uint8_t *out_buf, SgiState *s)
ret = expand_rle_row16(s, (uint16_t *)dest_row + z, linesize, s->depth);
if (ret != s->width)
return AVERROR_INVALIDDATA;
+ if (--remaining_lines == 0)
+ break;
+ dest_row -= s->linesize;
}
}
return 0;
@@ -204,7 +205,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
SgiState *s = avctx->priv_data;
unsigned int dimension, rle;
int ret = 0;
- uint8_t *out_buf, *out_end;
+ uint8_t *out_buf, *last_line;
bytestream2_init(&s->g, avpkt->data, avpkt->size);
if (bytestream2_get_bytes_left(&s->g) < SGI_HEADER_SIZE) {
@@ -258,14 +259,14 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
p->key_frame = 1;
out_buf = p->data[0];
- out_end = out_buf + p->linesize[0] * s->height;
+ last_line = out_buf + p->linesize[0] * (s->height - 1);
s->linesize = p->linesize[0];
/* Skip header. */
bytestream2_seek(&s->g, SGI_HEADER_SIZE, SEEK_SET);
if (rle) {
- ret = read_rle_sgi(out_end, s);
+ ret = read_rle_sgi(last_line, s);
} else {
ret = read_uncompressed_sgi(out_buf, s);
}
--
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] 11+ messages in thread* [FFmpeg-devel] [PATCH 3/7] avcodec/sgidec: Avoid redundant private context
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 2/7] avcodec/sgidec: Support negative linesizes Andreas Rheinhardt
@ 2022-09-30 17:05 ` Andreas Rheinhardt
2022-10-04 14:42 ` Tomas Härdin
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 4/7] avcodec/sgidec: Use planar pixel formats Andreas Rheinhardt
` (5 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-09-30 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
SGI is intra-frame only; the decoder therefore does not
maintain any state between frames, so remove
the private context.
Also rename depth to nb_components.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/sgidec.c | 158 ++++++++++++++++++++------------------------
1 file changed, 73 insertions(+), 85 deletions(-)
diff --git a/libavcodec/sgidec.c b/libavcodec/sgidec.c
index a449859bf8..bd49a3510d 100644
--- a/libavcodec/sgidec.c
+++ b/libavcodec/sgidec.c
@@ -25,25 +25,17 @@
#include "decode.h"
#include "sgi.h"
-typedef struct SgiState {
- AVCodecContext *avctx;
- unsigned int width;
- int height;
- unsigned int depth;
- unsigned int bytes_per_channel;
- int linesize;
- GetByteContext g;
-} SgiState;
-
/**
* Expand an RLE row into a channel.
- * @param s the current image state
+ * @param logctx a logcontext
* @param out_buf Points to one line after the output buffer.
+ * @param g GetByteContext used to read input from
* @param len length of out_buf in bytes
* @param pixelstride pixel stride of input buffer
* @return size of output in bytes, else return error code.
*/
-static int expand_rle_row8(SgiState *s, uint8_t *out_buf,
+static int expand_rle_row8(void *logctx, uint8_t *out_buf,
+ GetByteContext *g,
int len, int pixelstride)
{
unsigned char pixel, count;
@@ -51,26 +43,26 @@ static int expand_rle_row8(SgiState *s, uint8_t *out_buf,
uint8_t *out_end = out_buf + len;
while (out_buf < out_end) {
- if (bytestream2_get_bytes_left(&s->g) < 1)
+ if (bytestream2_get_bytes_left(g) < 1)
return AVERROR_INVALIDDATA;
- pixel = bytestream2_get_byteu(&s->g);
+ pixel = bytestream2_get_byteu(g);
if (!(count = (pixel & 0x7f))) {
break;
}
/* Check for buffer overflow. */
if (out_end - out_buf <= pixelstride * (count - 1)) {
- av_log(s->avctx, AV_LOG_ERROR, "Invalid pixel count.\n");
+ av_log(logctx, AV_LOG_ERROR, "Invalid pixel count.\n");
return AVERROR_INVALIDDATA;
}
if (pixel & 0x80) {
while (count--) {
- *out_buf = bytestream2_get_byte(&s->g);
+ *out_buf = bytestream2_get_byte(g);
out_buf += pixelstride;
}
} else {
- pixel = bytestream2_get_byte(&s->g);
+ pixel = bytestream2_get_byte(g);
while (count--) {
*out_buf = pixel;
@@ -81,7 +73,8 @@ static int expand_rle_row8(SgiState *s, uint8_t *out_buf,
return (out_buf - orig) / pixelstride;
}
-static int expand_rle_row16(SgiState *s, uint16_t *out_buf,
+static int expand_rle_row16(void *logctx, uint16_t *out_buf,
+ GetByteContext *g,
int len, int pixelstride)
{
unsigned short pixel;
@@ -90,26 +83,26 @@ static int expand_rle_row16(SgiState *s, uint16_t *out_buf,
uint16_t *out_end = out_buf + len;
while (out_buf < out_end) {
- if (bytestream2_get_bytes_left(&s->g) < 2)
+ if (bytestream2_get_bytes_left(g) < 2)
return AVERROR_INVALIDDATA;
- pixel = bytestream2_get_be16u(&s->g);
+ pixel = bytestream2_get_be16u(g);
if (!(count = (pixel & 0x7f)))
break;
/* Check for buffer overflow. */
if (out_end - out_buf <= pixelstride * (count - 1)) {
- av_log(s->avctx, AV_LOG_ERROR, "Invalid pixel count.\n");
+ av_log(logctx, AV_LOG_ERROR, "Invalid pixel count.\n");
return AVERROR_INVALIDDATA;
}
if (pixel & 0x80) {
while (count--) {
- pixel = bytestream2_get_ne16(&s->g);
+ pixel = bytestream2_get_ne16(g);
AV_WN16A(out_buf, pixel);
out_buf += pixelstride;
}
} else {
- pixel = bytestream2_get_ne16(&s->g);
+ pixel = bytestream2_get_ne16(g);
while (count--) {
AV_WN16A(out_buf, pixel);
@@ -127,34 +120,38 @@ static int expand_rle_row16(SgiState *s, uint16_t *out_buf,
* @param s the current image state
* @return 0 if no error, else return error code.
*/
-static int read_rle_sgi(uint8_t *last_line, SgiState *s)
+static int read_rle_sgi(void *logctx, uint8_t *last_line, GetByteContext *g,
+ ptrdiff_t stride, unsigned width, unsigned height,
+ unsigned nb_components, unsigned bytes_per_channel)
{
uint8_t *dest_row;
- unsigned int len = s->height * s->depth * 4;
- GetByteContext g_table = s->g;
+ unsigned int len = height * nb_components * 4;
+ GetByteContext g_table = *g;
unsigned int start_offset;
int linesize, ret;
/* size of RLE offset and length tables */
- if (len * 2 > bytestream2_get_bytes_left(&s->g)) {
+ if (len * 2 > bytestream2_get_bytes_left(g)) {
return AVERROR_INVALIDDATA;
}
- for (unsigned z = 0; z < s->depth; z++) {
+ for (unsigned z = 0; z < nb_components; z++) {
dest_row = last_line;
- for (int remaining_lines = s->height;;) {
- linesize = s->width * s->depth;
+ for (unsigned remaining_lines = height;;) {
+ linesize = width * nb_components;
start_offset = bytestream2_get_be32(&g_table);
- bytestream2_seek(&s->g, start_offset, SEEK_SET);
- if (s->bytes_per_channel == 1)
- ret = expand_rle_row8(s, dest_row + z, linesize, s->depth);
+ bytestream2_seek(g, start_offset, SEEK_SET);
+ if (bytes_per_channel == 1)
+ ret = expand_rle_row8(logctx, dest_row + z, g,
+ linesize, nb_components);
else
- ret = expand_rle_row16(s, (uint16_t *)dest_row + z, linesize, s->depth);
- if (ret != s->width)
+ ret = expand_rle_row16(logctx, (uint16_t *)dest_row + z, g,
+ linesize, nb_components);
+ if (ret != width)
return AVERROR_INVALIDDATA;
if (--remaining_lines == 0)
break;
- dest_row -= s->linesize;
+ dest_row -= stride;
}
}
return 0;
@@ -166,33 +163,34 @@ static int read_rle_sgi(uint8_t *last_line, SgiState *s)
* @param s the current image state
* @return 0 if read success, else return error code.
*/
-static int read_uncompressed_sgi(unsigned char *out_buf, SgiState *s)
+static int read_uncompressed_sgi(unsigned char *out_buf, GetByteContext *g,
+ ptrdiff_t stride, unsigned width, unsigned height,
+ unsigned nb_components, unsigned bytes_per_channel)
{
- int x, y, z;
- unsigned int offset = s->height * s->width * s->bytes_per_channel;
+ unsigned int offset = height * width * bytes_per_channel;
GetByteContext gp[4];
uint8_t *out_end;
/* Test buffer size. */
- if (offset * s->depth > bytestream2_get_bytes_left(&s->g))
+ if (offset * nb_components > bytestream2_get_bytes_left(g))
return AVERROR_INVALIDDATA;
/* Create a reader for each plane */
- for (z = 0; z < s->depth; z++) {
- gp[z] = s->g;
+ for (unsigned z = 0; z < nb_components; z++) {
+ gp[z] = *g;
bytestream2_skip(&gp[z], z * offset);
}
- for (y = s->height - 1; y >= 0; y--) {
- out_end = out_buf + (y * s->linesize);
- if (s->bytes_per_channel == 1) {
- for (x = s->width; x > 0; x--)
- for (z = 0; z < s->depth; z++)
+ for (int y = height - 1; y >= 0; y--) {
+ out_end = out_buf + y * stride;
+ if (bytes_per_channel == 1) {
+ for (unsigned x = width; x > 0; x--)
+ for (unsigned z = 0; z < nb_components; z++)
*out_end++ = bytestream2_get_byteu(&gp[z]);
} else {
uint16_t *out16 = (uint16_t *)out_end;
- for (x = s->width; x > 0; x--)
- for (z = 0; z < s->depth; z++)
+ for (unsigned x = width; x > 0; x--)
+ for (unsigned z = 0; z < nb_components; z++)
*out16++ = bytestream2_get_ne16u(&gp[z]);
}
}
@@ -202,31 +200,32 @@ static int read_uncompressed_sgi(unsigned char *out_buf, SgiState *s)
static int decode_frame(AVCodecContext *avctx, AVFrame *p,
int *got_frame, AVPacket *avpkt)
{
- SgiState *s = avctx->priv_data;
- unsigned int dimension, rle;
+ GetByteContext g;
+ unsigned int bytes_per_channel, nb_components, dimension, rle, width;
+ int height;
int ret = 0;
uint8_t *out_buf, *last_line;
- bytestream2_init(&s->g, avpkt->data, avpkt->size);
- if (bytestream2_get_bytes_left(&s->g) < SGI_HEADER_SIZE) {
+ bytestream2_init(&g, avpkt->data, avpkt->size);
+ if (bytestream2_get_bytes_left(&g) < SGI_HEADER_SIZE) {
av_log(avctx, AV_LOG_ERROR, "buf_size too small (%d)\n", avpkt->size);
return AVERROR_INVALIDDATA;
}
/* Test for SGI magic. */
- if (bytestream2_get_be16u(&s->g) != SGI_MAGIC) {
+ if (bytestream2_get_be16u(&g) != SGI_MAGIC) {
av_log(avctx, AV_LOG_ERROR, "bad magic number\n");
return AVERROR_INVALIDDATA;
}
- rle = bytestream2_get_byteu(&s->g);
- s->bytes_per_channel = bytestream2_get_byteu(&s->g);
- dimension = bytestream2_get_be16u(&s->g);
- s->width = bytestream2_get_be16u(&s->g);
- s->height = bytestream2_get_be16u(&s->g);
- s->depth = bytestream2_get_be16u(&s->g);
+ rle = bytestream2_get_byteu(&g);
+ bytes_per_channel = bytestream2_get_byteu(&g);
+ dimension = bytestream2_get_be16u(&g);
+ width = bytestream2_get_be16u(&g);
+ height = bytestream2_get_be16u(&g);
+ nb_components = bytestream2_get_be16u(&g);
- if (s->bytes_per_channel != 1 && s->bytes_per_channel != 2) {
+ if (bytes_per_channel != 1 && bytes_per_channel != 2) {
av_log(avctx, AV_LOG_ERROR, "wrong channel number\n");
return AVERROR_INVALIDDATA;
}
@@ -237,18 +236,18 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
return AVERROR_INVALIDDATA;
}
- if (s->depth == SGI_GRAYSCALE) {
- avctx->pix_fmt = s->bytes_per_channel == 2 ? AV_PIX_FMT_GRAY16BE : AV_PIX_FMT_GRAY8;
- } else if (s->depth == SGI_RGB) {
- avctx->pix_fmt = s->bytes_per_channel == 2 ? AV_PIX_FMT_RGB48BE : AV_PIX_FMT_RGB24;
- } else if (s->depth == SGI_RGBA) {
- avctx->pix_fmt = s->bytes_per_channel == 2 ? AV_PIX_FMT_RGBA64BE : AV_PIX_FMT_RGBA;
+ if (nb_components == SGI_GRAYSCALE) {
+ avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_GRAY16BE : AV_PIX_FMT_GRAY8;
+ } else if (nb_components == SGI_RGB) {
+ avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_RGB48BE : AV_PIX_FMT_RGB24;
+ } else if (nb_components == SGI_RGBA) {
+ avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_RGBA64BE : AV_PIX_FMT_RGBA;
} else {
av_log(avctx, AV_LOG_ERROR, "wrong picture format\n");
return AVERROR_INVALIDDATA;
}
- ret = ff_set_dimensions(avctx, s->width, s->height);
+ ret = ff_set_dimensions(avctx, width, height);
if (ret < 0)
return ret;
@@ -259,16 +258,16 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
p->key_frame = 1;
out_buf = p->data[0];
- last_line = out_buf + p->linesize[0] * (s->height - 1);
-
- s->linesize = p->linesize[0];
+ last_line = out_buf + p->linesize[0] * (height - 1);
/* Skip header. */
- bytestream2_seek(&s->g, SGI_HEADER_SIZE, SEEK_SET);
+ bytestream2_seek(&g, SGI_HEADER_SIZE, SEEK_SET);
if (rle) {
- ret = read_rle_sgi(last_line, s);
+ ret = read_rle_sgi(avctx, last_line, &g, p->linesize[0],
+ width, height, nb_components, bytes_per_channel);
} else {
- ret = read_uncompressed_sgi(out_buf, s);
+ ret = read_uncompressed_sgi(out_buf, &g, p->linesize[0],
+ width, height, nb_components, bytes_per_channel);
}
if (ret)
return ret;
@@ -277,22 +276,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
return avpkt->size;
}
-static av_cold int sgi_decode_init(AVCodecContext *avctx)
-{
- SgiState *s = avctx->priv_data;
-
- s->avctx = avctx;
-
- return 0;
-}
-
const FFCodec ff_sgi_decoder = {
.p.name = "sgi",
CODEC_LONG_NAME("SGI image"),
.p.type = AVMEDIA_TYPE_VIDEO,
.p.id = AV_CODEC_ID_SGI,
- .priv_data_size = sizeof(SgiState),
FF_CODEC_DECODE_CB(decode_frame),
- .init = sgi_decode_init,
.p.capabilities = AV_CODEC_CAP_DR1,
};
--
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] 11+ messages in thread* [FFmpeg-devel] [PATCH 4/7] avcodec/sgidec: Use planar pixel formats
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 2/7] avcodec/sgidec: Support negative linesizes Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 3/7] avcodec/sgidec: Avoid redundant private context Andreas Rheinhardt
@ 2022-09-30 17:05 ` Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 5/7] avcodec/c93: Fix segfault when using negative linesizes Andreas Rheinhardt
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-09-30 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
The data in SGI images is stored planar, so exporting
it via planar pixel formats is natural.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/sgidec.c | 123 ++++++++++++++++------------------
tests/ref/fate/sgi-rgb24 | 2 +-
tests/ref/fate/sgi-rgb24-rle | 2 +-
tests/ref/fate/sgi-rgb48 | 2 +-
tests/ref/fate/sgi-rgb48-rle | 2 +-
tests/ref/fate/sgi-rgba | 2 +-
tests/ref/fate/sgi-rgba-rle | 2 +-
tests/ref/fate/sgi-rgba64 | 2 +-
tests/ref/fate/sgi-rgba64-rle | 2 +-
tests/ref/lavf/sgi | 2 +-
10 files changed, 66 insertions(+), 75 deletions(-)
diff --git a/libavcodec/sgidec.c b/libavcodec/sgidec.c
index bd49a3510d..6ff2ee97f6 100644
--- a/libavcodec/sgidec.c
+++ b/libavcodec/sgidec.c
@@ -30,17 +30,15 @@
* @param logctx a logcontext
* @param out_buf Points to one line after the output buffer.
* @param g GetByteContext used to read input from
- * @param len length of out_buf in bytes
- * @param pixelstride pixel stride of input buffer
- * @return size of output in bytes, else return error code.
+ * @param width length of out_buf in nb of elements
+ * @return nb of elements written, else return error code.
*/
static int expand_rle_row8(void *logctx, uint8_t *out_buf,
- GetByteContext *g,
- int len, int pixelstride)
+ GetByteContext *g, unsigned width)
{
unsigned char pixel, count;
unsigned char *orig = out_buf;
- uint8_t *out_end = out_buf + len;
+ uint8_t *out_end = out_buf + width;
while (out_buf < out_end) {
if (bytestream2_get_bytes_left(g) < 1)
@@ -51,36 +49,31 @@ static int expand_rle_row8(void *logctx, uint8_t *out_buf,
}
/* Check for buffer overflow. */
- if (out_end - out_buf <= pixelstride * (count - 1)) {
+ if (out_end - out_buf < count) {
av_log(logctx, AV_LOG_ERROR, "Invalid pixel count.\n");
return AVERROR_INVALIDDATA;
}
if (pixel & 0x80) {
- while (count--) {
- *out_buf = bytestream2_get_byte(g);
- out_buf += pixelstride;
- }
+ while (count--)
+ *out_buf++ = bytestream2_get_byte(g);
} else {
pixel = bytestream2_get_byte(g);
- while (count--) {
- *out_buf = pixel;
- out_buf += pixelstride;
- }
+ while (count--)
+ *out_buf++ = pixel;
}
}
- return (out_buf - orig) / pixelstride;
+ return out_buf - orig;
}
static int expand_rle_row16(void *logctx, uint16_t *out_buf,
- GetByteContext *g,
- int len, int pixelstride)
+ GetByteContext *g, unsigned width)
{
unsigned short pixel;
unsigned char count;
unsigned short *orig = out_buf;
- uint16_t *out_end = out_buf + len;
+ uint16_t *out_end = out_buf + width;
while (out_buf < out_end) {
if (bytestream2_get_bytes_left(g) < 2)
@@ -90,7 +83,7 @@ static int expand_rle_row16(void *logctx, uint16_t *out_buf,
break;
/* Check for buffer overflow. */
- if (out_end - out_buf <= pixelstride * (count - 1)) {
+ if (out_end - out_buf < count) {
av_log(logctx, AV_LOG_ERROR, "Invalid pixel count.\n");
return AVERROR_INVALIDDATA;
}
@@ -99,18 +92,18 @@ static int expand_rle_row16(void *logctx, uint16_t *out_buf,
while (count--) {
pixel = bytestream2_get_ne16(g);
AV_WN16A(out_buf, pixel);
- out_buf += pixelstride;
+ out_buf++;
}
} else {
pixel = bytestream2_get_ne16(g);
while (count--) {
AV_WN16A(out_buf, pixel);
- out_buf += pixelstride;
+ out_buf++;
}
}
}
- return (out_buf - orig) / pixelstride;
+ return out_buf - orig;
}
@@ -120,15 +113,14 @@ static int expand_rle_row16(void *logctx, uint16_t *out_buf,
* @param s the current image state
* @return 0 if no error, else return error code.
*/
-static int read_rle_sgi(void *logctx, uint8_t *last_line, GetByteContext *g,
- ptrdiff_t stride, unsigned width, unsigned height,
+static int read_rle_sgi(void *logctx, uint8_t *out[4], ptrdiff_t stride[4],
+ GetByteContext *g, unsigned width, int height,
unsigned nb_components, unsigned bytes_per_channel)
{
- uint8_t *dest_row;
unsigned int len = height * nb_components * 4;
GetByteContext g_table = *g;
unsigned int start_offset;
- int linesize, ret;
+ int ret;
/* size of RLE offset and length tables */
if (len * 2 > bytestream2_get_bytes_left(g)) {
@@ -136,22 +128,19 @@ static int read_rle_sgi(void *logctx, uint8_t *last_line, GetByteContext *g,
}
for (unsigned z = 0; z < nb_components; z++) {
- dest_row = last_line;
- for (unsigned remaining_lines = height;;) {
- linesize = width * nb_components;
+ uint8_t *dest_row = out[z] + (height - 1) * stride[z];
+ while (1) {
start_offset = bytestream2_get_be32(&g_table);
bytestream2_seek(g, start_offset, SEEK_SET);
if (bytes_per_channel == 1)
- ret = expand_rle_row8(logctx, dest_row + z, g,
- linesize, nb_components);
+ ret = expand_rle_row8(logctx, dest_row, g, width);
else
- ret = expand_rle_row16(logctx, (uint16_t *)dest_row + z, g,
- linesize, nb_components);
+ ret = expand_rle_row16(logctx, (uint16_t *)dest_row, g, width);
if (ret != width)
return AVERROR_INVALIDDATA;
- if (--remaining_lines == 0)
+ if (dest_row == out[z])
break;
- dest_row -= stride;
+ dest_row -= stride[z];
}
}
return 0;
@@ -163,35 +152,23 @@ static int read_rle_sgi(void *logctx, uint8_t *last_line, GetByteContext *g,
* @param s the current image state
* @return 0 if read success, else return error code.
*/
-static int read_uncompressed_sgi(unsigned char *out_buf, GetByteContext *g,
- ptrdiff_t stride, unsigned width, unsigned height,
+static int read_uncompressed_sgi(uint8_t *const out[4], const ptrdiff_t stride[4],
+ GetByteContext *g, unsigned width, int height,
unsigned nb_components, unsigned bytes_per_channel)
{
- unsigned int offset = height * width * bytes_per_channel;
- GetByteContext gp[4];
- uint8_t *out_end;
+ unsigned rowsize = width * bytes_per_channel;
/* Test buffer size. */
- if (offset * nb_components > bytestream2_get_bytes_left(g))
+ if (rowsize * (int64_t)height > bytestream2_get_bytes_left(g))
return AVERROR_INVALIDDATA;
- /* Create a reader for each plane */
for (unsigned z = 0; z < nb_components; z++) {
- gp[z] = *g;
- bytestream2_skip(&gp[z], z * offset);
- }
-
- for (int y = height - 1; y >= 0; y--) {
- out_end = out_buf + y * stride;
- if (bytes_per_channel == 1) {
- for (unsigned x = width; x > 0; x--)
- for (unsigned z = 0; z < nb_components; z++)
- *out_end++ = bytestream2_get_byteu(&gp[z]);
- } else {
- uint16_t *out16 = (uint16_t *)out_end;
- for (unsigned x = width; x > 0; x--)
- for (unsigned z = 0; z < nb_components; z++)
- *out16++ = bytestream2_get_ne16u(&gp[z]);
+ uint8_t *cur_row = out[z] + (height - 1) * stride[z];
+ while (1) {
+ bytestream2_get_bufferu(g, cur_row, rowsize);
+ if (cur_row == out[z])
+ break;
+ cur_row -= stride[z];
}
}
return 0;
@@ -202,9 +179,10 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
{
GetByteContext g;
unsigned int bytes_per_channel, nb_components, dimension, rle, width;
+ uint8_t *out[4];
+ ptrdiff_t linesize[4];
int height;
int ret = 0;
- uint8_t *out_buf, *last_line;
bytestream2_init(&g, avpkt->data, avpkt->size);
if (bytestream2_get_bytes_left(&g) < SGI_HEADER_SIZE) {
@@ -239,9 +217,9 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
if (nb_components == SGI_GRAYSCALE) {
avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_GRAY16BE : AV_PIX_FMT_GRAY8;
} else if (nb_components == SGI_RGB) {
- avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_RGB48BE : AV_PIX_FMT_RGB24;
+ avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_GBRP16BE : AV_PIX_FMT_GBRP;
} else if (nb_components == SGI_RGBA) {
- avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_RGBA64BE : AV_PIX_FMT_RGBA;
+ avctx->pix_fmt = bytes_per_channel == 2 ? AV_PIX_FMT_GBRAP16BE : AV_PIX_FMT_GBRAP;
} else {
av_log(avctx, AV_LOG_ERROR, "wrong picture format\n");
return AVERROR_INVALIDDATA;
@@ -254,19 +232,32 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
if ((ret = ff_get_buffer(avctx, p, 0)) < 0)
return ret;
+ switch (nb_components) {
+#define MAP(in_idx, out_idx) \
+ out[(in_idx)] = p->data[(out_idx)]; \
+ linesize[(in_idx)] = p->linesize[(out_idx)]
+ case SGI_GRAYSCALE:
+ MAP(0, 0);
+ break;
+ case SGI_RGBA:
+ MAP(3, 3);
+ /* fallthrough */
+ case SGI_RGB:
+ MAP(0, 2);
+ MAP(1, 0);
+ MAP(2, 1);
+ break;
+ }
p->pict_type = AV_PICTURE_TYPE_I;
p->key_frame = 1;
- out_buf = p->data[0];
-
- last_line = out_buf + p->linesize[0] * (height - 1);
/* Skip header. */
bytestream2_seek(&g, SGI_HEADER_SIZE, SEEK_SET);
if (rle) {
- ret = read_rle_sgi(avctx, last_line, &g, p->linesize[0],
+ ret = read_rle_sgi(avctx, out, linesize, &g,
width, height, nb_components, bytes_per_channel);
} else {
- ret = read_uncompressed_sgi(out_buf, &g, p->linesize[0],
+ ret = read_uncompressed_sgi(out, linesize, &g,
width, height, nb_components, bytes_per_channel);
}
if (ret)
diff --git a/tests/ref/fate/sgi-rgb24 b/tests/ref/fate/sgi-rgb24
index 4326cabe00..238e3a1f26 100644
--- a/tests/ref/fate/sgi-rgb24
+++ b/tests/ref/fate/sgi-rgb24
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 393216, 0xa9b28fd9
+0, 0, 0, 1, 393216, 0xc8478fd9
diff --git a/tests/ref/fate/sgi-rgb24-rle b/tests/ref/fate/sgi-rgb24-rle
index d21bde15ba..25cc5f4737 100644
--- a/tests/ref/fate/sgi-rgb24-rle
+++ b/tests/ref/fate/sgi-rgb24-rle
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 393216, 0xe96e1de2
+0, 0, 0, 1, 393216, 0x7e231de2
diff --git a/tests/ref/fate/sgi-rgb48 b/tests/ref/fate/sgi-rgb48
index 29fe302514..3c59c782e5 100644
--- a/tests/ref/fate/sgi-rgb48
+++ b/tests/ref/fate/sgi-rgb48
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 786432, 0xee4aa667
+0, 0, 0, 1, 786432, 0xf9a2a667
diff --git a/tests/ref/fate/sgi-rgb48-rle b/tests/ref/fate/sgi-rgb48-rle
index 49fc973017..377f4577f1 100644
--- a/tests/ref/fate/sgi-rgb48-rle
+++ b/tests/ref/fate/sgi-rgb48-rle
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 786432, 0xbc743bc4
+0, 0, 0, 1, 786432, 0xec343bc4
diff --git a/tests/ref/fate/sgi-rgba b/tests/ref/fate/sgi-rgba
index 6a2d176582..dcdaf84ab2 100644
--- a/tests/ref/fate/sgi-rgba
+++ b/tests/ref/fate/sgi-rgba
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 524288, 0x4ee5adbb
+0, 0, 0, 1, 524288, 0x7401adbb
diff --git a/tests/ref/fate/sgi-rgba-rle b/tests/ref/fate/sgi-rgba-rle
index 6a2d176582..dcdaf84ab2 100644
--- a/tests/ref/fate/sgi-rgba-rle
+++ b/tests/ref/fate/sgi-rgba-rle
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 524288, 0x4ee5adbb
+0, 0, 0, 1, 524288, 0x7401adbb
diff --git a/tests/ref/fate/sgi-rgba64 b/tests/ref/fate/sgi-rgba64
index 00181dcb3b..4a28a29169 100644
--- a/tests/ref/fate/sgi-rgba64
+++ b/tests/ref/fate/sgi-rgba64
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 1048576, 0xc657e22b
+0, 0, 0, 1, 1048576, 0x1b61e22b
diff --git a/tests/ref/fate/sgi-rgba64-rle b/tests/ref/fate/sgi-rgba64-rle
index 354d391826..5610fc7fe6 100644
--- a/tests/ref/fate/sgi-rgba64-rle
+++ b/tests/ref/fate/sgi-rgba64-rle
@@ -3,4 +3,4 @@
#codec_id 0: rawvideo
#dimensions 0: 512x256
#sar 0: 0/1
-0, 0, 0, 1, 1048576, 0xb619d0f1
+0, 0, 0, 1, 1048576, 0x6122d0f1
diff --git a/tests/ref/lavf/sgi b/tests/ref/lavf/sgi
index ad27b805f0..bfab92c8a8 100644
--- a/tests/ref/lavf/sgi
+++ b/tests/ref/lavf/sgi
@@ -1,3 +1,3 @@
d446e540a7c18da5fd3cc0e9942cd46f *tests/data/images/sgi/02.sgi
307287 tests/data/images/sgi/02.sgi
-tests/data/images/sgi/%02d.sgi CRC=0x6da01946
+tests/data/images/sgi/%02d.sgi CRC=0x36e71946
--
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] 11+ messages in thread* [FFmpeg-devel] [PATCH 5/7] avcodec/c93: Fix segfault when using negative linesizes
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
` (2 preceding siblings ...)
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 4/7] avcodec/sgidec: Use planar pixel formats Andreas Rheinhardt
@ 2022-09-30 17:05 ` Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 6/7] avcodec/escape124: Fix segfault with " Andreas Rheinhardt
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-09-30 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
c93.c used an int for the stride and an unsigned for the current
linenumber. This does not work when using negative linesizes.
So use ptrdiff_t for stride and int for linenumber.
This fixes the cyberia-c93 FATE test when using negative linesizes.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/c93.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/libavcodec/c93.c b/libavcodec/c93.c
index 66b551a5d6..bfcbc7c150 100644
--- a/libavcodec/c93.c
+++ b/libavcodec/c93.c
@@ -130,7 +130,8 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
AVFrame * const oldpic = c93->pictures[c93->currentpic^1];
GetByteContext gb;
uint8_t *out;
- int stride, ret, i, x, y, b, bt = 0;
+ int ret, i, x, y, b, bt = 0;
+ ptrdiff_t stride;
if ((ret = ff_set_dimensions(avctx, WIDTH, HEIGHT)) < 0)
return ret;
@@ -156,7 +157,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
out = newpic->data[0] + y * stride;
for (x = 0; x < WIDTH; x += 8) {
uint8_t *copy_from = oldpic->data[0];
- unsigned int offset, j;
uint8_t cols[4], grps[4];
C93BlockType block_type;
@@ -165,16 +165,17 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
block_type= bt & 0x0F;
switch (block_type) {
- case C93_8X8_FROM_PREV:
- offset = bytestream2_get_le16(&gb);
+ case C93_8X8_FROM_PREV: {
+ int offset = bytestream2_get_le16(&gb);
if ((ret = copy_block(avctx, out, copy_from, offset, 8, stride)) < 0)
return ret;
break;
+ }
case C93_4X4_FROM_CURR:
copy_from = newpic->data[0];
case C93_4X4_FROM_PREV:
- for (j = 0; j < 8; j += 4) {
+ for (int j = 0; j < 8; j += 4) {
for (i = 0; i < 8; i += 4) {
int offset = bytestream2_get_le16(&gb);
int from_x = offset % WIDTH;
@@ -203,7 +204,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
case C93_4X4_2COLOR:
case C93_4X4_4COLOR:
case C93_4X4_4COLOR_GRP:
- for (j = 0; j < 8; j += 4) {
+ for (int j = 0; j < 8; j += 4) {
for (i = 0; i < 8; i += 4) {
if (block_type == C93_4X4_2COLOR) {
bytestream2_get_buffer(&gb, cols, 2);
@@ -226,7 +227,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
break;
case C93_8X8_INTRA:
- for (j = 0; j < 8; j++)
+ for (int j = 0; j < 8; j++)
bytestream2_get_buffer(&gb, out + j*stride, 8);
break;
--
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] 11+ messages in thread* [FFmpeg-devel] [PATCH 6/7] avcodec/escape124: Fix segfault with negative linesizes
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
` (3 preceding siblings ...)
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 5/7] avcodec/c93: Fix segfault when using negative linesizes Andreas Rheinhardt
@ 2022-09-30 17:05 ` Andreas Rheinhardt
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 7/7] avcodec/fraps: " Andreas Rheinhardt
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-09-30 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Use ptrdiff_t instead of unsigned for linesizes.
Fixes the armovie-escape124 FATE test with negative linesizes.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/escape124.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libavcodec/escape124.c b/libavcodec/escape124.c
index eeeb9bb0f7..024eec59ce 100644
--- a/libavcodec/escape124.c
+++ b/libavcodec/escape124.c
@@ -178,8 +178,8 @@ static void insert_mb_into_sb(SuperBlock* sb, MacroBlock mb, unsigned index) {
dst[4] = mb.pixels32[1];
}
-static void copy_superblock(uint16_t* dest, unsigned dest_stride,
- uint16_t* src, unsigned src_stride)
+static void copy_superblock(uint16_t* dest, ptrdiff_t dest_stride,
+ uint16_t* src, ptrdiff_t src_stride)
{
unsigned y;
if (src)
@@ -211,7 +211,7 @@ static int escape124_decode_frame(AVCodecContext *avctx, AVFrame *frame,
superblocks_per_row = avctx->width / 8, skip = -1;
uint16_t* old_frame_data, *new_frame_data;
- unsigned old_stride, new_stride;
+ ptrdiff_t old_stride, new_stride;
int ret;
--
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] 11+ messages in thread* [FFmpeg-devel] [PATCH 7/7] avcodec/fraps: Fix segfault with negative linesizes
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
` (4 preceding siblings ...)
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 6/7] avcodec/escape124: Fix segfault with " Andreas Rheinhardt
@ 2022-09-30 17:05 ` Andreas Rheinhardt
2022-10-03 22:05 ` [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
2022-10-04 0:03 ` James Almer
7 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-09-30 17:05 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Using unsigned and negative linesizes doesn't really work.
Use ptrdiff_t instead. This fixes the fraps-v0 and fraps-v1
FATE tests with negative linesizes.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/fraps.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
index 9c8cbf7323..4c4c46b602 100644
--- a/libavcodec/fraps.c
+++ b/libavcodec/fraps.c
@@ -141,7 +141,6 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *f,
int buf_size = avpkt->size;
uint32_t header;
unsigned int version,header_size;
- unsigned int x, y;
const uint32_t *buf32;
uint32_t *luma1,*luma2,*cb,*cr;
uint32_t offs[4];
@@ -238,12 +237,12 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *f,
}
buf32 = (const uint32_t*)buf;
- for (y = 0; y < avctx->height / 2; y++) {
+ for (ptrdiff_t y = 0; y < avctx->height / 2; y++) {
luma1 = (uint32_t*)&f->data[0][ y * 2 * f->linesize[0] ];
luma2 = (uint32_t*)&f->data[0][ (y * 2 + 1) * f->linesize[0] ];
cr = (uint32_t*)&f->data[1][ y * f->linesize[1] ];
cb = (uint32_t*)&f->data[2][ y * f->linesize[2] ];
- for (x = 0; x < avctx->width; x += 8) {
+ for (ptrdiff_t x = 0; x < avctx->width; x += 8) {
*luma1++ = *buf32++;
*luma1++ = *buf32++;
*luma2++ = *buf32++;
@@ -258,18 +257,18 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *f,
if (is_pal) {
uint32_t *pal = (uint32_t *)f->data[1];
- for (y = 0; y < 256; y++) {
+ for (unsigned y = 0; y < 256; y++) {
pal[y] = AV_RL32(buf) | 0xFF000000;
buf += 4;
}
- for (y = 0; y <avctx->height; y++)
+ for (ptrdiff_t y = 0; y < avctx->height; y++)
memcpy(&f->data[0][y * f->linesize[0]],
&buf[y * avctx->width],
avctx->width);
} else {
/* Fraps v1 is an upside-down BGR24 */
- for (y = 0; y<avctx->height; y++)
+ for (ptrdiff_t y = 0; y < avctx->height; y++)
memcpy(&f->data[0][(avctx->height - y - 1) * f->linesize[0]],
&buf[y * avctx->width * 3],
3 * avctx->width);
--
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] 11+ messages in thread* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
` (5 preceding siblings ...)
2022-09-30 17:05 ` [FFmpeg-devel] [PATCH 7/7] avcodec/fraps: " Andreas Rheinhardt
@ 2022-10-03 22:05 ` Andreas Rheinhardt
2022-10-04 0:03 ` James Almer
7 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-10-03 22:05 UTC (permalink / raw)
To: ffmpeg-devel
Andreas Rheinhardt:
> Fixes segfaults with negative linesizes; in particular,
> this affected the sunraster-(1|8|24)bit-(raw|rle) and
> sunraster-8bit_gray-raw FATE tests.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavcodec/sunrast.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/sunrast.c b/libavcodec/sunrast.c
> index 77feef06e1..45b29e4d72 100644
> --- a/libavcodec/sunrast.c
> +++ b/libavcodec/sunrast.c
> @@ -31,7 +31,8 @@ static int sunrast_decode_frame(AVCodecContext *avctx, AVFrame *p,
> {
> const uint8_t *buf = avpkt->data;
> const uint8_t *buf_end = avpkt->data + avpkt->size;
> - unsigned int w, h, depth, type, maptype, maplength, stride, x, y, len, alen;
> + unsigned int w, h, depth, type, maptype, maplength, x, y, len, alen;
> + ptrdiff_t stride;
> uint8_t *ptr, *ptr2 = NULL;
> const uint8_t *bufstart = buf;
> int ret;
> @@ -141,7 +142,7 @@ static int sunrast_decode_frame(AVCodecContext *avctx, AVFrame *p,
>
> if (type == RT_BYTE_ENCODED) {
> int value, run;
> - uint8_t *end = ptr + h * stride;
> + uint8_t *end = ptr + (ptrdiff_t)h * stride;
>
> x = 0;
> while (ptr != end && buf < buf_end) {
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] 11+ messages in thread* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride
2022-09-30 17:02 [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
` (6 preceding siblings ...)
2022-10-03 22:05 ` [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride Andreas Rheinhardt
@ 2022-10-04 0:03 ` James Almer
2022-10-04 0:25 ` Andreas Rheinhardt
7 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2022-10-04 0:03 UTC (permalink / raw)
To: ffmpeg-devel
On 9/30/2022 2:02 PM, Andreas Rheinhardt wrote:
> Fixes segfaults with negative linesizes; in particular,
> this affected the sunraster-(1|8|24)bit-(raw|rle) and
> sunraster-8bit_gray-raw FATE tests.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavcodec/sunrast.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/sunrast.c b/libavcodec/sunrast.c
> index 77feef06e1..45b29e4d72 100644
> --- a/libavcodec/sunrast.c
> +++ b/libavcodec/sunrast.c
> @@ -31,7 +31,8 @@ static int sunrast_decode_frame(AVCodecContext *avctx, AVFrame *p,
> {
> const uint8_t *buf = avpkt->data;
> const uint8_t *buf_end = avpkt->data + avpkt->size;
> - unsigned int w, h, depth, type, maptype, maplength, stride, x, y, len, alen;
> + unsigned int w, h, depth, type, maptype, maplength, x, y, len, alen;
> + ptrdiff_t stride;
> uint8_t *ptr, *ptr2 = NULL;
> const uint8_t *bufstart = buf;
> int ret;
> @@ -141,7 +142,7 @@ static int sunrast_decode_frame(AVCodecContext *avctx, AVFrame *p,
>
> if (type == RT_BYTE_ENCODED) {
> int value, run;
> - uint8_t *end = ptr + h * stride;
> + uint8_t *end = ptr + (ptrdiff_t)h * stride;
Is the cast needed if stride is already ptrdiff_t?
>
> x = 0;
> while (ptr != end && buf < buf_end) {
_______________________________________________
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] 11+ messages in thread* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/sunrast: Use ptrdiff_t for stride
2022-10-04 0:03 ` James Almer
@ 2022-10-04 0:25 ` Andreas Rheinhardt
0 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-10-04 0:25 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> On 9/30/2022 2:02 PM, Andreas Rheinhardt wrote:
>> Fixes segfaults with negative linesizes; in particular,
>> this affected the sunraster-(1|8|24)bit-(raw|rle) and
>> sunraster-8bit_gray-raw FATE tests.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> libavcodec/sunrast.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/sunrast.c b/libavcodec/sunrast.c
>> index 77feef06e1..45b29e4d72 100644
>> --- a/libavcodec/sunrast.c
>> +++ b/libavcodec/sunrast.c
>> @@ -31,7 +31,8 @@ static int sunrast_decode_frame(AVCodecContext
>> *avctx, AVFrame *p,
>> {
>> const uint8_t *buf = avpkt->data;
>> const uint8_t *buf_end = avpkt->data + avpkt->size;
>> - unsigned int w, h, depth, type, maptype, maplength, stride, x, y,
>> len, alen;
>> + unsigned int w, h, depth, type, maptype, maplength, x, y, len, alen;
>> + ptrdiff_t stride;
>> uint8_t *ptr, *ptr2 = NULL;
>> const uint8_t *bufstart = buf;
>> int ret;
>> @@ -141,7 +142,7 @@ static int sunrast_decode_frame(AVCodecContext
>> *avctx, AVFrame *p,
>> if (type == RT_BYTE_ENCODED) {
>> int value, run;
>> - uint8_t *end = ptr + h * stride;
>> + uint8_t *end = ptr + (ptrdiff_t)h * stride;
>
> Is the cast needed if stride is already ptrdiff_t?
>
Yes. If ptrdiff_t is the signed counterpart of h (an unsigned; i.e. on
32bit systems), h * stride would otherwise be evaluated as an unsigned.
And in this case, it would be a huge positive value, so that the pointer
arithmetic would be undefined. It would nevertheless work in practice
with a flat address space, because the value would be correct modulo
2^32 (modulo SIZE_MAX + 1).
- Andreas
PS: This "it would nevertheless work in practice is not uncommon". E.g.
when using types other than uint8_t to access the frames, we often use
something like "uint16_t *ptr = (uint16_t*)frame->data[0] +
(frame->linesize[0] / sizeof(*ptr)) * h". frame->linesize[0] /
sizeof(*ptr) will (usually) be performed in size_t, with negative
linesizes promoted to huge values. frame->linesize[0] / sizeof(*ptr)
will then be in the lower half of size_t (because the division uses a
logical shift). But given that pointer arithmetic is performed on units
of the pointed-to type, the offset frame->linesize[0] / sizeof(*ptr)
will be multiplied by sizeof(*ptr) and this happens to work correctly
modulo SIZE_MAX + 1 on ordinary systems.
We should nevertheless replace this idiom by "(uint16_t*)(frame->data[0]
+ frame->linesize[0] * h)".
Btw: At lots of places we presuppose that linesize is always divisible
by the size of the type we use to access the data. Yet we only require
linesize to be sufficiently aligned according to the CPU's maxiumum. In
case a CPU has no alignment requirement whatsoever it would be legal to
use any number as linesize, even when it does not divide
sizeof(uint16_t), sizeof(float) etc.
_______________________________________________
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] 11+ messages in thread