[FFmpeg-devel] [PATCH v2 1/6] avcodec/flashsvenc: Fix packet size calculation

[FFmpeg-devel] [PATCH v2 1/6] avcodec/flashsvenc: Fix packet size calculation

[Andreas Rheinhardt]

The earlier code did not account for the frame header as well
as the block headers; furthermore, in case a large part of
a block is unused (due to padding), the output size may
exceed 3 * width * height (where the dimensions correspond
to the visible pixels) due to the overhead of the zlib header,
so use the padded dimensions to calculate the maximum packet size
(which is also what the actual call to compress2() uses).
Fixes ticket #10053.

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

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index 35793400fa..4cedb53fe4 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -54,11 +54,15 @@
 #include "put_bits.h"
 #include "bytestream.h"
 
+/* These values are hardcoded for now. */
+#define BLOCK_WIDTH  (4 * 16U)
+#define BLOCK_HEIGHT (4 * 16U)
 
 typedef struct FlashSVContext {
     AVCodecContext *avctx;
     uint8_t        *previous_frame;
     int             image_width, image_height;
+    unsigned        packet_size;
     int             block_width, block_height;
     uint8_t        *encbuffer;
     int             block_size;
@@ -100,6 +104,7 @@ static av_cold int flashsv_encode_end(AVCodecContext *avctx)
 static av_cold int flashsv_encode_init(AVCodecContext *avctx)
 {
     FlashSVContext *s = avctx->priv_data;
+    int h_blocks, v_blocks, nb_blocks;
 
     s->avctx = avctx;
 
@@ -114,6 +119,11 @@ static av_cold int flashsv_encode_init(AVCodecContext *avctx)
     s->image_width  = avctx->width;
     s->image_height = avctx->height;
 
+    h_blocks = (s->image_width  + BLOCK_WIDTH - 1) / BLOCK_WIDTH;
+    v_blocks = (s->image_height + BLOCK_WIDTH - 1) / BLOCK_WIDTH;
+    nb_blocks = h_blocks * v_blocks;
+    s->packet_size = 4 + nb_blocks * (2 + 3 * BLOCK_WIDTH * BLOCK_HEIGHT);
+
     s->encbuffer = av_mallocz(s->image_width * s->image_height * 3);
 
     if (!s->encbuffer) {
@@ -229,7 +239,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, s->packet_size);
+    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 v2 2/6] 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 4cedb53fe4..d627b12351 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -64,7 +64,6 @@ typedef struct FlashSVContext {
     int             image_width, image_height;
     unsigned        packet_size;
     int             block_width, block_height;
-    uint8_t        *encbuffer;
     int             block_size;
     int             last_key_frame;
     uint8_t         tmpblock[3 * 256 * 256];
@@ -95,7 +94,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;
@@ -124,13 +122,6 @@ static av_cold int flashsv_encode_init(AVCodecContext *avctx)
     nb_blocks = h_blocks * v_blocks;
     s->packet_size = 4 + nb_blocks * (2 + 3 * BLOCK_WIDTH * BLOCK_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 v2 3/6] 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 d627b12351..bd912fb401 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -70,15 +70,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];
@@ -128,7 +127,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;
@@ -204,7 +203,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 v2 4/6] 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 bd912fb401..ba693ca8f3 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -209,7 +209,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 v2 5/6] 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 ba693ca8f3..c4b16dff84 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"
@@ -60,7 +61,8 @@
 
 typedef struct FlashSVContext {
     AVCodecContext *avctx;
-    uint8_t        *previous_frame;
+    const uint8_t  *previous_frame;
+    AVBufferRef    *prev_frame_buf;
     int             image_width, image_height;
     unsigned        packet_size;
     int             block_width, block_height;
@@ -93,7 +95,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;
 }
@@ -203,26 +205,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) {
@@ -234,15 +227,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) {
@@ -254,6 +239,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 v2 6/6] avcodec/flashsvenc: Remove unnecessary or unused variable

[Andreas Rheinhardt]

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

diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
index c4b16dff84..3a35876d9c 100644
--- a/libavcodec/flashsvenc.c
+++ b/libavcodec/flashsvenc.c
@@ -65,8 +65,6 @@ typedef struct FlashSVContext {
     AVBufferRef    *prev_frame_buf;
     int             image_width, image_height;
     unsigned        packet_size;
-    int             block_width, block_height;
-    int             block_size;
     int             last_key_frame;
     uint8_t         tmpblock[3 * 256 * 256];
 } FlashSVContext;
@@ -204,7 +202,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;
     const uint8_t *prev_frame = s->previous_frame;
     int res;
     int I_frame = 0;
@@ -226,7 +223,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,
                                  prev_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".

Re: [FFmpeg-devel] [PATCH v2 1/6] avcodec/flashsvenc: Fix packet size calculation

[Andreas Rheinhardt]

Andreas Rheinhardt:
> The earlier code did not account for the frame header as well
> as the block headers; furthermore, in case a large part of
> a block is unused (due to padding), the output size may
> exceed 3 * width * height (where the dimensions correspond
> to the visible pixels) due to the overhead of the zlib header,
> so use the padded dimensions to calculate the maximum packet size
> (which is also what the actual call to compress2() uses).
> Fixes ticket #10053.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/flashsvenc.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/flashsvenc.c b/libavcodec/flashsvenc.c
> index 35793400fa..4cedb53fe4 100644
> --- a/libavcodec/flashsvenc.c
> +++ b/libavcodec/flashsvenc.c
> @@ -54,11 +54,15 @@
>  #include "put_bits.h"
>  #include "bytestream.h"
>  
> +/* These values are hardcoded for now. */
> +#define BLOCK_WIDTH  (4 * 16U)
> +#define BLOCK_HEIGHT (4 * 16U)
>  
>  typedef struct FlashSVContext {
>      AVCodecContext *avctx;
>      uint8_t        *previous_frame;
>      int             image_width, image_height;
> +    unsigned        packet_size;
>      int             block_width, block_height;
>      uint8_t        *encbuffer;
>      int             block_size;
> @@ -100,6 +104,7 @@ static av_cold int flashsv_encode_end(AVCodecContext *avctx)
>  static av_cold int flashsv_encode_init(AVCodecContext *avctx)
>  {
>      FlashSVContext *s = avctx->priv_data;
> +    int h_blocks, v_blocks, nb_blocks;
>  
>      s->avctx = avctx;
>  
> @@ -114,6 +119,11 @@ static av_cold int flashsv_encode_init(AVCodecContext *avctx)
>      s->image_width  = avctx->width;
>      s->image_height = avctx->height;
>  
> +    h_blocks = (s->image_width  + BLOCK_WIDTH - 1) / BLOCK_WIDTH;
> +    v_blocks = (s->image_height + BLOCK_WIDTH - 1) / BLOCK_WIDTH;
> +    nb_blocks = h_blocks * v_blocks;
> +    s->packet_size = 4 + nb_blocks * (2 + 3 * BLOCK_WIDTH * BLOCK_HEIGHT);
> +
>      s->encbuffer = av_mallocz(s->image_width * s->image_height * 3);
>  
>      if (!s->encbuffer) {
> @@ -229,7 +239,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, s->packet_size);
> +    if (res < 0)
>          return res;
>  
>      pkt->size = encode_bitstream(s, p, pkt->data, pkt->size, opt_w * 16, opt_h * 16,

Will apply this patchset tomorrow 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".