* [FFmpeg-devel] [PATCH 2/7] avcodec/pgxdec: Avoid always-false checks
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
@ 2022-04-24 4:42 ` Andreas Rheinhardt
2022-04-24 11:18 ` Paul B Mahol
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 3/7] avcodec/pgxdec: Remove pointless checks Andreas Rheinhardt
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Andreas Rheinhardt @ 2022-04-24 4:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
We have already checked that there is data to be read.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pgxdec.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c
index 154a683b4f..9c474036da 100644
--- a/libavcodec/pgxdec.c
+++ b/libavcodec/pgxdec.c
@@ -32,9 +32,9 @@ static int pgx_get_number(AVCodecContext *avctx, GetByteContext *g, int *number)
*number = 0;
while (1) {
uint64_t temp;
- if (!bytestream2_get_bytes_left(g))
+ if (bytestream2_get_bytes_left(g) <= 0)
return AVERROR_INVALIDDATA;
- digit = bytestream2_get_byte(g);
+ digit = bytestream2_get_byteu(g);
if (digit == ' ' || digit == 0xA || digit == 0xD)
break;
else if (digit < '0' || digit > '9')
@@ -59,22 +59,22 @@ static int pgx_decode_header(AVCodecContext *avctx, GetByteContext *g,
if (bytestream2_get_bytes_left(g) < 12)
return AVERROR_INVALIDDATA;
- bytestream2_skip(g, 6);
+ bytestream2_skipu(g, 6);
// Is the component signed?
- byte = bytestream2_peek_byte(g);
+ byte = bytestream2_peek_byteu(g);
if (byte == '+') {
*sign = 0;
- bytestream2_skip(g, 1);
+ bytestream2_skipu(g, 1);
} else if (byte == '-') {
*sign = 1;
- bytestream2_skip(g, 1);
+ bytestream2_skipu(g, 1);
} else if (byte == 0)
goto error;
- byte = bytestream2_peek_byte(g);
+ byte = bytestream2_peek_byteu(g);
if (byte == ' ')
- bytestream2_skip(g, 1);
+ bytestream2_skipu(g, 1);
else if (byte == 0)
goto error;
@@ -104,9 +104,9 @@ error:
for (j = 0; j < width; j++) { \
unsigned val; \
if (sign) \
- val = (PIXEL)bytestream2_get_ ##suffix(g) + (1 << (depth - 1)); \
+ val = (PIXEL)bytestream2_get_ ##suffix##u(g) + (1 << (depth - 1)); \
else \
- val = bytestream2_get_ ##suffix(g); \
+ val = bytestream2_get_ ##suffix##u(g); \
val <<= (D - depth); \
*(line + j) = val; \
} \
--
2.32.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] 14+ messages in thread* [FFmpeg-devel] [PATCH 3/7] avcodec/pgxdec: Remove pointless checks
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 2/7] avcodec/pgxdec: Avoid always-false checks Andreas Rheinhardt
@ 2022-04-24 4:42 ` Andreas Rheinhardt
2022-04-24 11:17 ` Paul B Mahol
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 4/7] avcodec/pgxdec: Fix issue with negative linesizes Andreas Rheinhardt
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Andreas Rheinhardt @ 2022-04-24 4:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
These checks were (most likely) added to check for overreads
as the bytestream2_get_* functions return 0 in this case.
Yet this is not necessary anymore as we now have an explicit check
for the size. Should the input contain a real \0, pgx_get_number()
will error out lateron.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pgxdec.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c
index 9c474036da..c9ada5afb5 100644
--- a/libavcodec/pgxdec.c
+++ b/libavcodec/pgxdec.c
@@ -69,14 +69,11 @@ static int pgx_decode_header(AVCodecContext *avctx, GetByteContext *g,
} else if (byte == '-') {
*sign = 1;
bytestream2_skipu(g, 1);
- } else if (byte == 0)
- goto error;
+ }
byte = bytestream2_peek_byteu(g);
if (byte == ' ')
bytestream2_skipu(g, 1);
- else if (byte == 0)
- goto error;
if (pgx_get_number(avctx, g, depth))
goto error;
--
2.32.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] 14+ messages in thread* [FFmpeg-devel] [PATCH 4/7] avcodec/pgxdec: Fix issue with negative linesizes
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 2/7] avcodec/pgxdec: Avoid always-false checks Andreas Rheinhardt
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 3/7] avcodec/pgxdec: Remove pointless checks Andreas Rheinhardt
@ 2022-04-24 4:42 ` Andreas Rheinhardt
2022-04-24 11:15 ` Paul B Mahol
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 5/7] avcodec/pgxdec: Hoist branch out of loop Andreas Rheinhardt
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Andreas Rheinhardt @ 2022-04-24 4:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
The PGX decoder accesses the lines via code like
(PIXEL*)frame->data[0] + i*frame->linesize[0]/sizeof(PIXEL)
where PIXEL is a macro parameter. This code has issues with negative
linesizes, because the type of sizeof(PIXEL) is size_t, so
that on common systems i*linesize/sizeof(PIXEL) will
always be an unsigned type that is very large in case linesize is
negative. This happens to work*, but it is undefined behaviour
and e.g. leads to "src/libavcodec/pgxdec.c:114:1: runtime error:
addition of unsigned offset to 0x7efe9c2b7040 overflowed to 0x7efe9c2b6040"
errors from UBSAN.
Fix this by using (PIXEL*)(frame->data[0] + i*frame->linesize[0]).
This is allowed because linesize has to be suitably aligned.
*: Converting a negative int to size_t works by adding SIZE_MAX + 1
to the number, so that the result is off by (SIZE_MAX + 1) /
sizeof(PIXEL). Converting the pointer arithmetic (performed on PIXELs)
back to ordinary pointers is tantamount to multiplying by sizeof(PIXEL),
so that the result is off by SIZE_MAX + 1; but SIZE_MAX + 1 == 0
for the underlying pointers.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pgxdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c
index c9ada5afb5..30895b51ee 100644
--- a/libavcodec/pgxdec.c
+++ b/libavcodec/pgxdec.c
@@ -97,7 +97,7 @@ error:
{ \
int i, j; \
for (i = 0; i < height; i++) { \
- PIXEL *line = (PIXEL*)frame->data[0] + i*frame->linesize[0]/sizeof(PIXEL); \
+ PIXEL *line = (PIXEL*)(frame->data[0] + i * frame->linesize[0]); \
for (j = 0; j < width; j++) { \
unsigned val; \
if (sign) \
--
2.32.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] 14+ messages in thread* [FFmpeg-devel] [PATCH 5/7] avcodec/pgxdec: Hoist branch out of loop
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
` (2 preceding siblings ...)
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 4/7] avcodec/pgxdec: Fix issue with negative linesizes Andreas Rheinhardt
@ 2022-04-24 4:42 ` Andreas Rheinhardt
2022-04-24 11:16 ` Paul B Mahol
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 6/7] avcodec/pgxdec: Use unsigned types for unsigned values Andreas Rheinhardt
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Andreas Rheinhardt @ 2022-04-24 4:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pgxdec.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c
index 30895b51ee..29de103167 100644
--- a/libavcodec/pgxdec.c
+++ b/libavcodec/pgxdec.c
@@ -95,16 +95,13 @@ error:
static inline void write_frame_ ##D(AVFrame *frame, GetByteContext *g, \
int width, int height, int sign, int depth) \
{ \
+ const unsigned offset = sign ? (1 << (D - 1)) : 0; \
int i, j; \
for (i = 0; i < height; i++) { \
PIXEL *line = (PIXEL*)(frame->data[0] + i * frame->linesize[0]); \
for (j = 0; j < width; j++) { \
- unsigned val; \
- if (sign) \
- val = (PIXEL)bytestream2_get_ ##suffix##u(g) + (1 << (depth - 1)); \
- else \
- val = bytestream2_get_ ##suffix##u(g); \
- val <<= (D - depth); \
+ unsigned val = bytestream2_get_ ##suffix##u(g) << (D - depth); \
+ val ^= offset; \
*(line + j) = val; \
} \
} \
--
2.32.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] 14+ messages in thread* [FFmpeg-devel] [PATCH 6/7] avcodec/pgxdec: Use unsigned types for unsigned values
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
` (3 preceding siblings ...)
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 5/7] avcodec/pgxdec: Hoist branch out of loop Andreas Rheinhardt
@ 2022-04-24 4:42 ` Andreas Rheinhardt
2022-04-24 11:16 ` Paul B Mahol
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 7/7] avcodec/xfacedec: Add AV_CODEC_CAP_DR1 Andreas Rheinhardt
2022-04-24 11:18 ` [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Paul B Mahol
6 siblings, 1 reply; 14+ messages in thread
From: Andreas Rheinhardt @ 2022-04-24 4:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Both AV_PIX_FMT_GRAY8 and AV_PIX_FMT_GRAY16 use unsigned values,
not signed ones. The fact that the input might be signed
in some cases in the original format doesn't change this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pgxdec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c
index 29de103167..52e2c2a36c 100644
--- a/libavcodec/pgxdec.c
+++ b/libavcodec/pgxdec.c
@@ -107,8 +107,8 @@ error:
} \
} \
-WRITE_FRAME(8, int8_t, byte)
-WRITE_FRAME(16, int16_t, be16)
+WRITE_FRAME(8, uint8_t, byte)
+WRITE_FRAME(16, uint16_t, be16)
static int pgx_decode_frame(AVCodecContext *avctx, AVFrame *p,
int *got_frame, AVPacket *avpkt)
--
2.32.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] 14+ messages in thread* [FFmpeg-devel] [PATCH 7/7] avcodec/xfacedec: Add AV_CODEC_CAP_DR1
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
` (4 preceding siblings ...)
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 6/7] avcodec/pgxdec: Use unsigned types for unsigned values Andreas Rheinhardt
@ 2022-04-24 4:42 ` Andreas Rheinhardt
2022-04-24 11:12 ` Paul B Mahol
2022-04-24 11:18 ` [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Paul B Mahol
6 siblings, 1 reply; 14+ messages in thread
From: Andreas Rheinhardt @ 2022-04-24 4:42 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
This decoder uses ff_get_buffer() and does nothing weird
(it does not even rely on any alignment of the frame's data/linesize).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/xfacedec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavcodec/xfacedec.c b/libavcodec/xfacedec.c
index b103b5beda..f15bc2d773 100644
--- a/libavcodec/xfacedec.c
+++ b/libavcodec/xfacedec.c
@@ -180,6 +180,7 @@ const FFCodec ff_xface_decoder = {
.p.long_name = NULL_IF_CONFIG_SMALL("X-face image"),
.p.type = AVMEDIA_TYPE_VIDEO,
.p.id = AV_CODEC_ID_XFACE,
+ .p.capabilities = AV_CODEC_CAP_DR1,
.priv_data_size = sizeof(XFaceContext),
.init = xface_decode_init,
FF_CODEC_DECODE_CB(xface_decode_frame),
--
2.32.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] 14+ messages in thread* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check
2022-04-24 4:39 [FFmpeg-devel] [PATCH 1/7] avcodec/pgxdec: Make better use of size check Andreas Rheinhardt
` (5 preceding siblings ...)
2022-04-24 4:42 ` [FFmpeg-devel] [PATCH 7/7] avcodec/xfacedec: Add AV_CODEC_CAP_DR1 Andreas Rheinhardt
@ 2022-04-24 11:18 ` Paul B Mahol
6 siblings, 0 replies; 14+ messages in thread
From: Paul B Mahol @ 2022-04-24 11:18 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Andreas Rheinhardt
lgtm if tested
_______________________________________________
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] 14+ messages in thread