[FFmpeg-devel] [PATCH 1/7] avcodec/pnmenc: Check av_image_get_buffer_size()

[FFmpeg-devel] [PATCH 1/7] avcodec/pnmenc: Check av_image_get_buffer_size()

[Andreas Rheinhardt]

Fixes the crash in ticket #10050.
Also ensure that we don't overflow before ff_get_encode_buffer().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/pnmenc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavcodec/pnmenc.c b/libavcodec/pnmenc.c
index 9eb663306d..c998dd410c 100644
--- a/libavcodec/pnmenc.c
+++ b/libavcodec/pnmenc.c
@@ -42,7 +42,10 @@ static int pnm_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     int size = av_image_get_buffer_size(avctx->pix_fmt,
                                         avctx->width, avctx->height, 1);
 
-    if ((ret = ff_get_encode_buffer(avctx, pkt, size + 200, 0)) < 0)
+    if (size < 0)
+        return size;
+
+    if ((ret = ff_get_encode_buffer(avctx, pkt, size + 200U, 0)) < 0)
         return ret;
 
     bytestream_start =
-- 
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".

[FFmpeg-devel] [PATCH 2/7] avcodec/flashsvenc: Account for header in packet size

[Andreas Rheinhardt]

Fixes ticket #10053.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index 35793400fa..9d065bb92d 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -229,7 +229,8 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
         I_frame = 1;
     }
 
-    if ((res = ff_alloc_packet(avctx, pkt, s->image_width * s->image_height * 3)) < 0)
+    res = ff_alloc_packet(avctx, pkt, 4U + s->image_width * s->image_height * 3);
+    if (res < 0)
         return res;
 
     pkt->size = encode_bitstream(s, p, pkt->data, pkt->size, opt_w * 16, opt_h * 16,
-- 
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".

[FFmpeg-devel] [PATCH 3/7] avcodec/flashsvenc: Remove unused buffer

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index 9d065bb92d..0618b2aa1b 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -60,7 +60,6 @@ typedef struct FlashSVContext {
     uint8_t        *previous_frame;
     int             image_width, image_height;
     int             block_width, block_height;
-    uint8_t        *encbuffer;
     int             block_size;
     int             last_key_frame;
     uint8_t         tmpblock[3 * 256 * 256];
@@ -91,7 +90,6 @@ static av_cold int flashsv_encode_end(AVCodecContext *avctx)
 {
     FlashSVContext *s = avctx->priv_data;
 
-    av_freep(&s->encbuffer);
     av_freep(&s->previous_frame);
 
     return 0;
@@ -114,13 +112,6 @@ static av_cold int flashsv_encode_init(AVCodecContext *avctx)
     s->image_width  = avctx->width;
     s->image_height = avctx->height;
 
-    s->encbuffer = av_mallocz(s->image_width * s->image_height * 3);
-
-    if (!s->encbuffer) {
-        av_log(avctx, AV_LOG_ERROR, "Memory allocation failed.\n");
-        return AVERROR(ENOMEM);
-    }
-
     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".

[FFmpeg-devel] [PATCH 4/7] avcodec/flashsvenc: Use const where appropriate

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index 0618b2aa1b..18dfde0e24 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -66,15 +66,14 @@ typedef struct FlashSVContext {
 } FlashSVContext;
 
 static int copy_region_enc(const uint8_t *sptr, uint8_t *dptr, int dx, int dy,
-                           int h, int w, int stride, uint8_t *pfptr)
+                           int h, int w, int stride, const uint8_t *pfptr)
 {
     int i, j;
-    uint8_t *npfptr;
     int diff = 0;
 
     for (i = dx + h; i > dx; i--) {
         const uint8_t *nsptr = sptr + i * stride + dy * 3;
-        npfptr = pfptr + i * stride + dy * 3;
+        const uint8_t *npfptr = pfptr + i * stride + dy * 3;
         for (j = 0; j < w * 3; j++) {
             diff    |= npfptr[j] ^ nsptr[j];
             dptr[j]  = nsptr[j];
@@ -118,7 +117,7 @@ static av_cold int flashsv_encode_init(AVCodecContext *avctx)
 
 static int encode_bitstream(FlashSVContext *s, const AVFrame *p, uint8_t *buf,
                             int buf_size, int block_width, int block_height,
-                            uint8_t *previous_frame, int *I_frame)
+                            const uint8_t *previous_frame, int *I_frame)
 {
 
     PutBitContext pb;
@@ -194,7 +193,7 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 {
     FlashSVContext * const s = avctx->priv_data;
     const AVFrame * const p = pict;
-    uint8_t *pfptr;
+    const uint8_t *pfptr;
     int res;
     int I_frame = 0;
     int opt_w = 4, opt_h = 4;
-- 
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".

[FFmpeg-devel] [PATCH 5/7] avcodec/flashsvenc: Change check for first frame

[Andreas Rheinhardt]

AVCodecContext.frame_number is actually only incremented
in case encoding was successfull; if e.g. the ff_alloc_packet()
below fails, it won't be incremented and therefore it is possible
for the previous_frame buffer to be allocated for multiple
first frames, leaking every one except the last.
So check for whether there already is a previous frame instead.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index 18dfde0e24..f42ef73b37 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -199,7 +199,7 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     int opt_w = 4, opt_h = 4;
 
     /* First frame needs to be a keyframe */
-    if (avctx->frame_number == 0) {
+    if (!s->previous_frame) {
         s->previous_frame = av_mallocz(FFABS(p->linesize[0]) * s->image_height);
         if (!s->previous_frame) {
             av_log(avctx, AV_LOG_ERROR, "Memory allocation failed.\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".

[FFmpeg-devel] [PATCH 6/7] avcodec/flashsvenc: Avoid copying frame

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index f42ef73b37..3c3d9c8813 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -48,6 +48,8 @@
 #include <stdlib.h>
 #include <zlib.h>
 
+#include "libavutil/buffer.h"
+
 #include "avcodec.h"
 #include "codec_internal.h"
 #include "encode.h"
@@ -58,6 +60,7 @@
 typedef struct FlashSVContext {
     AVCodecContext *avctx;
     uint8_t        *previous_frame;
+    AVBufferRef    *prev_frame_buf;
     int             image_width, image_height;
     int             block_width, block_height;
     int             block_size;
@@ -89,7 +92,7 @@ static av_cold int flashsv_encode_end(AVCodecContext *avctx)
 {
     FlashSVContext *s = avctx->priv_data;
 
-    av_freep(&s->previous_frame);
+    av_buffer_unref(&s->prev_frame_buf);
 
     return 0;
 }
@@ -193,26 +196,19 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 {
     FlashSVContext * const s = avctx->priv_data;
     const AVFrame * const p = pict;
-    const uint8_t *pfptr;
     int res;
     int I_frame = 0;
     int opt_w = 4, opt_h = 4;
 
     /* First frame needs to be a keyframe */
-    if (!s->previous_frame) {
-        s->previous_frame = av_mallocz(FFABS(p->linesize[0]) * s->image_height);
-        if (!s->previous_frame) {
-            av_log(avctx, AV_LOG_ERROR, "Memory allocation failed.\n");
+    if (!s->prev_frame_buf) {
+        s->prev_frame_buf = av_buffer_ref(pict->buf[0]);
+        if (!s->prev_frame_buf)
             return AVERROR(ENOMEM);
-        }
+        s->previous_frame = pict->data[0];
         I_frame = 1;
     }
 
-    if (p->linesize[0] < 0)
-        pfptr = s->previous_frame - (s->image_height - 1) * p->linesize[0];
-    else
-        pfptr = s->previous_frame;
-
     /* Check the placement of keyframes */
     if (avctx->gop_size > 0 &&
         avctx->frame_number >= s->last_key_frame + avctx->gop_size) {
@@ -224,15 +220,7 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
         return res;
 
     pkt->size = encode_bitstream(s, p, pkt->data, pkt->size, opt_w * 16, opt_h * 16,
-                                 pfptr, &I_frame);
-
-    //save the current frame
-    if (p->linesize[0] > 0)
-        memcpy(s->previous_frame, p->data[0], s->image_height * p->linesize[0]);
-    else
-        memcpy(s->previous_frame,
-               p->data[0] + p->linesize[0] * (s->image_height - 1),
-               s->image_height * FFABS(p->linesize[0]));
+                                 s->previous_frame, &I_frame);
 
     //mark the frame type so the muxer can mux it correctly
     if (I_frame) {
@@ -244,6 +232,12 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
         pkt->flags |= AV_PKT_FLAG_KEY;
     *got_packet = 1;
 
+    //save the current frame
+    res = av_buffer_replace(&s->prev_frame_buf, pict->buf[0]);
+    if (res < 0)
+        return res;
+    s->previous_frame = pict->data[0];
+
     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".

[FFmpeg-devel] [PATCH 7/7] avcodec/flashsvenc: Avoid unnecessary variable

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index 3c3d9c8813..89119733df 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -195,7 +195,6 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
                                 const AVFrame *pict, int *got_packet)
 {
     FlashSVContext * const s = avctx->priv_data;
-    const AVFrame * const p = pict;
     int res;
     int I_frame = 0;
     int opt_w = 4, opt_h = 4;
@@ -219,7 +218,8 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     if (res < 0)
         return res;
 
-    pkt->size = encode_bitstream(s, p, pkt->data, pkt->size, opt_w * 16, opt_h * 16,
+    pkt->size = encode_bitstream(s, pict, pkt->data, pkt->size,
+                                 opt_w * 16, opt_h * 16,
                                  s->previous_frame, &I_frame);
 
     //mark the frame type so the muxer can mux it correctly
-- 
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".

[FFmpeg-devel] [PATCH v2 6/7] avcodec/flashsvenc: Avoid copying frame

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/flashsvenc.c | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index f42ef73b37..f3d95c3fe5 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -44,10 +44,11 @@
  * Speed up. Make the difference check faster.
  */
 
-#include <stdio.h>
-#include <stdlib.h>
+#include <stdint.h>
 #include <zlib.h>
 
+#include "libavutil/buffer.h"
+
 #include "avcodec.h"
 #include "codec_internal.h"
 #include "encode.h"
@@ -57,7 +58,8 @@
 
 typedef struct FlashSVContext {
     AVCodecContext *avctx;
-    uint8_t        *previous_frame;
+    const uint8_t  *previous_frame;
+    AVBufferRef    *prev_frame_buf;
     int             image_width, image_height;
     int             block_width, block_height;
     int             block_size;
@@ -89,7 +91,7 @@ static av_cold int flashsv_encode_end(AVCodecContext *avctx)
 {
     FlashSVContext *s = avctx->priv_data;
 
-    av_freep(&s->previous_frame);
+    av_buffer_unref(&s->prev_frame_buf);
 
     return 0;
 }
@@ -193,26 +195,17 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 {
     FlashSVContext * const s = avctx->priv_data;
     const AVFrame * const p = pict;
-    const uint8_t *pfptr;
+    const uint8_t *prev_frame = s->previous_frame;
     int res;
     int I_frame = 0;
     int opt_w = 4, opt_h = 4;
 
     /* First frame needs to be a keyframe */
     if (!s->previous_frame) {
-        s->previous_frame = av_mallocz(FFABS(p->linesize[0]) * s->image_height);
-        if (!s->previous_frame) {
-            av_log(avctx, AV_LOG_ERROR, "Memory allocation failed.\n");
-            return AVERROR(ENOMEM);
-        }
+        prev_frame = pict->data[0];
         I_frame = 1;
     }
 
-    if (p->linesize[0] < 0)
-        pfptr = s->previous_frame - (s->image_height - 1) * p->linesize[0];
-    else
-        pfptr = s->previous_frame;
-
     /* Check the placement of keyframes */
     if (avctx->gop_size > 0 &&
         avctx->frame_number >= s->last_key_frame + avctx->gop_size) {
@@ -224,15 +217,7 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
         return res;
 
     pkt->size = encode_bitstream(s, p, pkt->data, pkt->size, opt_w * 16, opt_h * 16,
-                                 pfptr, &I_frame);
-
-    //save the current frame
-    if (p->linesize[0] > 0)
-        memcpy(s->previous_frame, p->data[0], s->image_height * p->linesize[0]);
-    else
-        memcpy(s->previous_frame,
-               p->data[0] + p->linesize[0] * (s->image_height - 1),
-               s->image_height * FFABS(p->linesize[0]));
+                                 prev_frame, &I_frame);
 
     //mark the frame type so the muxer can mux it correctly
     if (I_frame) {
@@ -244,6 +229,12 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
         pkt->flags |= AV_PKT_FLAG_KEY;
     *got_packet = 1;
 
+    //save the current frame
+    res = av_buffer_replace(&s->prev_frame_buf, pict->buf[0]);
+    if (res < 0)
+        return res;
+    s->previous_frame = pict->data[0];
+
     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".

Re: [FFmpeg-devel] [PATCH 2/7] avcodec/flashsvenc: Account for header in packet size

[James Almer]

On 11/18/2022 11:59 PM, Andreas Rheinhardt wrote:
> Fixes ticket #10053.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/flashsvenc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
> index 35793400fa..9d065bb92d 100644
> --- a/libavcodec/flashsvenc.c
> +++ b/libavcodec/flashsvenc.c
> @@ -229,7 +229,8 @@ static int flashsv_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>           I_frame = 1;
>       }
>   
> -    if ((res = ff_alloc_packet(avctx, pkt, s->image_width * s->image_height * 3)) < 0)
> +    res = ff_alloc_packet(avctx, pkt, 4U + s->image_width * s->image_height * 3);

For a 1x1 image (like the one from the ticket) this results in 7 bytes + 
padding being allocated...

> +    if (res < 0)
>           return res;
>   
>       pkt->size = encode_bitstream(s, p, pkt->data, pkt->size, opt_w * 16, opt_h * 16,

...yet encode_bitstream() will return 17, meaning it wrote into the 
padding bytes, which just happens to work because said padding was big 
enough. The smallest dimension that results in this being equal or 
smaller than the allocated size is 3x3.
_______________________________________________
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".