[FFmpeg-devel] [PATCH 1/7] avcodec/assenc: Use size_t for length of string

[FFmpeg-devel] [PATCH 1/7] avcodec/assenc: Use size_t for length of string

[Andreas Rheinhardt]

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

diff --git a/libavcodec/assenc.c b/libavcodec/assenc.c
index e548b9fd3b..e734522729 100644
--- a/libavcodec/assenc.c
+++ b/libavcodec/assenc.c
@@ -45,7 +45,7 @@ static int ass_encode_frame(AVCodecContext *avctx,
                             unsigned char *buf, int bufsize,
                             const AVSubtitle *sub)
 {
-    int len;
+    size_t len;
 
     if (sub->num_rects != 1) {
         av_log(avctx, AV_LOG_ERROR, "Only one rect per AVSubtitle is supported in ASS.\n");
@@ -59,7 +59,7 @@ static int ass_encode_frame(AVCodecContext *avctx,
 
     len = av_strlcpy(buf, sub->rects[0]->ass, bufsize);
 
-    if (len > bufsize - 1) {
+    if (len >= bufsize) {
         av_log(avctx, AV_LOG_ERROR, "Buffer too small for ASS event.\n");
         return AVERROR_BUFFER_TOO_SMALL;
     }
-- 
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/srtenc: Don't copy data around unnecessarily

[Andreas Rheinhardt]

Using av_bprint_init_for_buffer() avoids copying data
into the internal AVBPrint buffer (or worse: to allocate
a temporary buffer in case the internal buffer does not
suffice). It also ensures that the data is always
0-terminated, whereas the current code never does this
and returns success in case the length of the
string to write and the size of the buffer coincide.

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

diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
index 7e36a2200c..9b6f61ebd1 100644
--- a/libavcodec/srtenc.c
+++ b/libavcodec/srtenc.c
@@ -23,7 +23,6 @@
 
 #include <stdarg.h>
 #include "avcodec.h"
-#include "libavutil/avstring.h"
 #include "libavutil/bprint.h"
 #include "ass_split.h"
 #include "ass.h"
@@ -138,7 +137,6 @@ static av_cold int srt_encode_init(AVCodecContext *avctx)
     SRTContext *s = avctx->priv_data;
     s->avctx = avctx;
     s->ass_ctx = ff_ass_split(avctx->subtitle_header);
-    av_bprint_init(&s->buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
     return s->ass_ctx ? 0 : AVERROR_INVALIDDATA;
 }
 
@@ -237,7 +235,7 @@ static int encode_frame(AVCodecContext *avctx,
     ASSDialog *dialog;
     int i;
 
-    av_bprint_clear(&s->buffer);
+    av_bprint_init_for_buffer(&s->buffer, buf, bufsize);
 
     for (i=0; i<sub->num_rects; i++) {
         const char *ass = sub->rects[i]->ass;
@@ -257,16 +255,13 @@ static int encode_frame(AVCodecContext *avctx,
         ff_ass_free_dialog(&dialog);
     }
 
-    if (!av_bprint_is_complete(&s->buffer))
-        return AVERROR(ENOMEM);
     if (!s->buffer.len)
         return 0;
 
-    if (s->buffer.len > bufsize) {
+    if (!av_bprint_is_complete(&s->buffer)) {
         av_log(avctx, AV_LOG_ERROR, "Buffer too small for ASS event.\n");
         return AVERROR_BUFFER_TOO_SMALL;
     }
-    memcpy(buf, s->buffer.str, s->buffer.len);
 
     return s->buffer.len;
 }
@@ -287,7 +282,6 @@ static int srt_encode_close(AVCodecContext *avctx)
 {
     SRTContext *s = avctx->priv_data;
     ff_ass_split_free(s->ass_ctx);
-    av_bprint_finalize(&s->buffer, NULL);
     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 3/7] avcodec/webvttenc: Don't copy data around unnecessarily

[Andreas Rheinhardt]

Using av_bprint_init_for_buffer() avoids copying data
into the internal AVBPrint buffer (or worse: to allocate
a temporary buffer in case the internal buffer does not
suffice). It also ensures that the data is always
0-terminated.

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

diff --git a/libavcodec/webvttenc.c b/libavcodec/webvttenc.c
index 4369aacb74..9922f33485 100644
--- a/libavcodec/webvttenc.c
+++ b/libavcodec/webvttenc.c
@@ -22,7 +22,6 @@
 
 #include <stdarg.h>
 #include "avcodec.h"
-#include "libavutil/avstring.h"
 #include "libavutil/bprint.h"
 #include "ass_split.h"
 #include "ass.h"
@@ -162,7 +161,7 @@ static int webvtt_encode_frame(AVCodecContext *avctx,
     ASSDialog *dialog;
     int i;
 
-    av_bprint_clear(&s->buffer);
+    av_bprint_init_for_buffer(&s->buffer, buf, bufsize);
 
     for (i=0; i<sub->num_rects; i++) {
         const char *ass = sub->rects[i]->ass;
@@ -180,16 +179,13 @@ static int webvtt_encode_frame(AVCodecContext *avctx,
         ff_ass_free_dialog(&dialog);
     }
 
-    if (!av_bprint_is_complete(&s->buffer))
-        return AVERROR(ENOMEM);
     if (!s->buffer.len)
         return 0;
 
-    if (s->buffer.len > bufsize) {
+    if (!av_bprint_is_complete(&s->buffer)) {
         av_log(avctx, AV_LOG_ERROR, "Buffer too small for ASS event.\n");
         return AVERROR_BUFFER_TOO_SMALL;
     }
-    memcpy(buf, s->buffer.str, s->buffer.len);
 
     return s->buffer.len;
 }
@@ -198,7 +194,6 @@ static int webvtt_encode_close(AVCodecContext *avctx)
 {
     WebVTTContext *s = avctx->priv_data;
     ff_ass_split_free(s->ass_ctx);
-    av_bprint_finalize(&s->buffer, NULL);
     return 0;
 }
 
@@ -207,7 +202,6 @@ static av_cold int webvtt_encode_init(AVCodecContext *avctx)
     WebVTTContext *s = avctx->priv_data;
     s->avctx = avctx;
     s->ass_ctx = ff_ass_split(avctx->subtitle_header);
-    av_bprint_init(&s->buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
     return s->ass_ctx ? 0 : AVERROR_INVALIDDATA;
 }
 
-- 
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/ttmlenc: Don't copy data around unnecessarily

[Andreas Rheinhardt]

Using av_bprint_init_for_buffer() avoids copying data
into the internal AVBPrint buffer (or worse: to allocate
a temporary buffer in case the internal buffer does not
suffice).

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

diff --git a/libavcodec/ttmlenc.c b/libavcodec/ttmlenc.c
index 6a2ab23cab..f48274d2ea 100644
--- a/libavcodec/ttmlenc.c
+++ b/libavcodec/ttmlenc.c
@@ -29,11 +29,9 @@
 
 #include "avcodec.h"
 #include "codec_internal.h"
-#include "libavutil/avstring.h"
 #include "libavutil/bprint.h"
 #include "libavutil/internal.h"
 #include "ass_split.h"
-#include "ass.h"
 #include "ttmlenc.h"
 
 typedef struct {
@@ -84,7 +82,7 @@ static int ttml_encode_frame(AVCodecContext *avctx, uint8_t *buf,
     ASSDialog *dialog;
     int i;
 
-    av_bprint_clear(&s->buffer);
+    av_bprint_init_for_buffer(&s->buffer, buf, bufsize);
 
     for (i=0; i<sub->num_rects; i++) {
         const char *ass = sub->rects[i]->ass;
@@ -129,14 +127,9 @@ static int ttml_encode_frame(AVCodecContext *avctx, uint8_t *buf,
         ff_ass_free_dialog(&dialog);
     }
 
-    if (!av_bprint_is_complete(&s->buffer))
-        return AVERROR(ENOMEM);
     if (!s->buffer.len)
         return 0;
-
-    // force null-termination, so in case our destination buffer is
-    // too small, the return value is larger than bufsize minus null.
-    if (av_strlcpy(buf, s->buffer.str, bufsize) > bufsize - 1) {
+    if (!av_bprint_is_complete(&s->buffer)) {
         av_log(avctx, AV_LOG_ERROR, "Buffer too small for TTML event.\n");
         return AVERROR_BUFFER_TOO_SMALL;
     }
@@ -150,8 +143,6 @@ static av_cold int ttml_encode_close(AVCodecContext *avctx)
 
     ff_ass_split_free(s->ass_ctx);
 
-    av_bprint_finalize(&s->buffer, NULL);
-
     return 0;
 }
 
@@ -306,6 +297,7 @@ static int ttml_write_header_content(AVCodecContext *avctx)
     const size_t base_extradata_size = TTMLENC_EXTRADATA_SIGNATURE_SIZE + 1 +
                                        AV_INPUT_BUFFER_PADDING_SIZE;
     size_t additional_extradata_size = 0;
+    int ret;
 
     if (script_info.play_res_x <= 0 || script_info.play_res_y <= 0) {
         av_log(avctx, AV_LOG_ERROR,
@@ -314,6 +306,8 @@ static int ttml_write_header_content(AVCodecContext *avctx)
         return AVERROR_INVALIDDATA;
     }
 
+    av_bprint_init(&s->buffer, 0, INT_MAX - base_extradata_size);
+
     // write the first string in extradata, attributes in the base "tt" element.
     av_bprintf(&s->buffer, TTML_DEFAULT_NAMESPACING);
     // the cell resolution is in character cells, so not exactly 1:1 against
@@ -329,10 +323,10 @@ static int ttml_write_header_content(AVCodecContext *avctx)
     av_bprintf(&s->buffer, "    <layout>\n");
 
     for (int i = 0; i < ass->styles_count; i++) {
-        int ret = ttml_write_region(avctx, &s->buffer, script_info,
-                                    ass->styles[i]);
+        ret = ttml_write_region(avctx, &s->buffer, script_info,
+                                ass->styles[i]);
         if (ret < 0)
-            return ret;
+            goto fail;
     }
 
     av_bprintf(&s->buffer, "    </layout>\n");
@@ -340,14 +334,16 @@ static int ttml_write_header_content(AVCodecContext *avctx)
     av_bprint_chars(&s->buffer, '\0', 1);
 
     if (!av_bprint_is_complete(&s->buffer)) {
-        return AVERROR(ENOMEM);
+        ret = AVERROR(ENOMEM);
+        goto fail;
     }
 
     additional_extradata_size = s->buffer.len;
 
     if (!(avctx->extradata =
             av_mallocz(base_extradata_size + additional_extradata_size))) {
-        return AVERROR(ENOMEM);
+        ret = AVERROR(ENOMEM);
+        goto fail;
     }
 
     avctx->extradata_size =
@@ -359,9 +355,11 @@ static int ttml_write_header_content(AVCodecContext *avctx)
         memcpy(avctx->extradata + TTMLENC_EXTRADATA_SIGNATURE_SIZE,
                s->buffer.str, additional_extradata_size);
 
-    av_bprint_clear(&s->buffer);
+    ret = 0;
+fail:
+    av_bprint_finalize(&s->buffer, NULL);
 
-    return 0;
+    return ret;
 }
 
 static av_cold int ttml_encode_init(AVCodecContext *avctx)
@@ -370,8 +368,6 @@ static av_cold int ttml_encode_init(AVCodecContext *avctx)
     int ret = AVERROR_BUG;
     s->avctx   = avctx;
 
-    av_bprint_init(&s->buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
-
     if (!(s->ass_ctx = ff_ass_split(avctx->subtitle_header))) {
         return AVERROR_INVALIDDATA;
     }
-- 
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/ttmlenc: Remove always-true check

[Andreas Rheinhardt]

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

diff --git a/libavcodec/ttmlenc.c b/libavcodec/ttmlenc.c
index f48274d2ea..779cb75199 100644
--- a/libavcodec/ttmlenc.c
+++ b/libavcodec/ttmlenc.c
@@ -351,9 +351,8 @@ static int ttml_write_header_content(AVCodecContext *avctx)
     memcpy(avctx->extradata, TTMLENC_EXTRADATA_SIGNATURE,
            TTMLENC_EXTRADATA_SIGNATURE_SIZE);
 
-    if (additional_extradata_size)
-        memcpy(avctx->extradata + TTMLENC_EXTRADATA_SIGNATURE_SIZE,
-               s->buffer.str, additional_extradata_size);
+    memcpy(avctx->extradata + TTMLENC_EXTRADATA_SIGNATURE_SIZE,
+           s->buffer.str, additional_extradata_size);
 
     ret = 0;
 fail:
-- 
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/srtenc, webvttenc: Use av_printf_format

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/srtenc.c    | 5 +----
 libavcodec/webvttenc.c | 5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
index 9b6f61ebd1..a607beb990 100644
--- a/libavcodec/srtenc.c
+++ b/libavcodec/srtenc.c
@@ -41,10 +41,7 @@ typedef struct {
 } SRTContext;
 
 
-#ifdef __GNUC__
-__attribute__ ((__format__ (__printf__, 2, 3)))
-#endif
-static void srt_print(SRTContext *s, const char *str, ...)
+static av_printf_format(2, 3) void srt_print(SRTContext *s, const char *str, ...)
 {
     va_list vargs;
     va_start(vargs, str);
diff --git a/libavcodec/webvttenc.c b/libavcodec/webvttenc.c
index 9922f33485..3b66b4f2dd 100644
--- a/libavcodec/webvttenc.c
+++ b/libavcodec/webvttenc.c
@@ -38,10 +38,7 @@ typedef struct {
     int stack_ptr;
 } WebVTTContext;
 
-#ifdef __GNUC__
-__attribute__ ((__format__ (__printf__, 2, 3)))
-#endif
-static void webvtt_print(WebVTTContext *s, const char *str, ...)
+static av_printf_format(2, 3) void webvtt_print(WebVTTContext *s, const char *str, ...)
 {
     va_list vargs;
     va_start(vargs, str);
-- 
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/movtextenc: Don't copy data around unnecessarily

[Andreas Rheinhardt]

Using av_bprint_init_for_buffer() avoids copying data
into the internal AVBPrint buffer (or worse: to allocate
a temporary buffer in case the internal buffer does not
suffice).

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

diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
index 7aa74d7c9d..fd8c7dc9f7 100644
--- a/libavcodec/movtextenc.c
+++ b/libavcodec/movtextenc.c
@@ -22,7 +22,6 @@
 #include <stdarg.h>
 #include "avcodec.h"
 #include "libavutil/opt.h"
-#include "libavutil/avstring.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mem.h"
 #include "libavutil/common.h"
@@ -170,7 +169,6 @@ static int mov_text_encode_close(AVCodecContext *avctx)
     ff_ass_split_free(s->ass_ctx);
     av_freep(&s->style_attributes);
     av_freep(&s->fonts);
-    av_bprint_finalize(&s->buffer, NULL);
     return 0;
 }
 
@@ -183,6 +181,9 @@ static int encode_sample_description(AVCodecContext *avctx)
     int font_names_total_len = 0;
     MovTextContext *s = avctx->priv_data;
     uint8_t buf[30], *p = buf;
+    int ret;
+
+    av_bprint_init(&s->buffer, 0, INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE + 1);
 
     //  0x00, 0x00, 0x00, 0x00, // uint32_t displayFlags
     //  0x01,                   // int8_t horizontal-justification
@@ -306,19 +307,23 @@ static int encode_sample_description(AVCodecContext *avctx)
     //     };
 
     if (!av_bprint_is_complete(&s->buffer)) {
-        return AVERROR(ENOMEM);
+        ret = AVERROR(ENOMEM);
+        goto fail;
     }
 
     avctx->extradata_size = s->buffer.len;
     avctx->extradata = av_mallocz(avctx->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
     if (!avctx->extradata) {
-        return AVERROR(ENOMEM);
+        ret = AVERROR(ENOMEM);
+        goto fail;
     }
 
     memcpy(avctx->extradata, s->buffer.str, avctx->extradata_size);
-    av_bprint_clear(&s->buffer);
+    ret = 0;
+fail:
+    av_bprint_finalize(&s->buffer, NULL);
 
-    return 0;
+    return ret;
 }
 
 static av_cold int mov_text_encode_init(AVCodecContext *avctx)
@@ -327,8 +332,6 @@ static av_cold int mov_text_encode_init(AVCodecContext *avctx)
     MovTextContext *s = avctx->priv_data;
     s->avctx = avctx;
 
-    av_bprint_init(&s->buffer, 0, AV_BPRINT_SIZE_UNLIMITED);
-
     s->ass_ctx = ff_ass_split(avctx->subtitle_header);
     if (!s->ass_ctx)
         return AVERROR_INVALIDDATA;
@@ -640,10 +643,14 @@ static int mov_text_encode_frame(AVCodecContext *avctx, unsigned char *buf,
     ASSDialog *dialog;
     int i, length;
 
+    if (bufsize < 3)
+        goto too_small;
+
     s->text_pos = 0;
     s->count = 0;
     s->box_flags = 0;
-    av_bprint_clear(&s->buffer);
+
+    av_bprint_init_for_buffer(&s->buffer, buf + 2, bufsize - 2);
     for (i = 0; i < sub->num_rects; i++) {
         const char *ass = sub->rects[i]->ass;
 
@@ -663,23 +670,19 @@ static int mov_text_encode_frame(AVCodecContext *avctx, unsigned char *buf,
     if (s->buffer.len > UINT16_MAX)
         return AVERROR(ERANGE);
     AV_WB16(buf, s->buffer.len);
-    buf += 2;
 
     for (size_t j = 0; j < box_count; j++)
         box_types[j].encode(s);
 
-    if (!av_bprint_is_complete(&s->buffer))
-        return AVERROR(ENOMEM);
-
     if (!s->buffer.len)
         return 0;
 
-    if (s->buffer.len > bufsize - 3) {
+    if (!av_bprint_is_complete(&s->buffer)) {
+too_small:
         av_log(avctx, AV_LOG_ERROR, "Buffer too small for ASS event.\n");
         return AVERROR_BUFFER_TOO_SMALL;
     }
 
-    memcpy(buf, s->buffer.str, s->buffer.len);
     length = s->buffer.len + 2;
 
     return length;
-- 
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 1/7] avcodec/assenc: Use size_t for length of string

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/assenc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/assenc.c b/libavcodec/assenc.c
> index e548b9fd3b..e734522729 100644
> --- a/libavcodec/assenc.c
> +++ b/libavcodec/assenc.c
> @@ -45,7 +45,7 @@ static int ass_encode_frame(AVCodecContext *avctx,
>                              unsigned char *buf, int bufsize,
>                              const AVSubtitle *sub)
>  {
> -    int len;
> +    size_t len;
>  
>      if (sub->num_rects != 1) {
>          av_log(avctx, AV_LOG_ERROR, "Only one rect per AVSubtitle is supported in ASS.\n");
> @@ -59,7 +59,7 @@ static int ass_encode_frame(AVCodecContext *avctx,
>  
>      len = av_strlcpy(buf, sub->rects[0]->ass, bufsize);
>  
> -    if (len > bufsize - 1) {
> +    if (len >= bufsize) {
>          av_log(avctx, AV_LOG_ERROR, "Buffer too small for ASS event.\n");
>          return AVERROR_BUFFER_TOO_SMALL;
>      }

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".