diff mbox series

[FFmpeg-devel,v2] avcodec/mpeg12dec: switch to AVBufferRef buffer for a53 caption

Message ID 1592746212-28284-1-git-send-email-lance.lmwang@gmail.com
State Accepted
Commit a705bcd763e344fac191e157ffeddc285388b7fa
Headers show
Series [FFmpeg-devel,v2] avcodec/mpeg12dec: switch to AVBufferRef buffer for a53 caption | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang June 21, 2020, 1:30 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
It's old patch, rebase with master, now have tested with the mpeg2 sample
for ticket 6105, the output srt is same.

 libavcodec/mpeg12dec.c | 85 ++++++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 45 deletions(-)

Comments

Lance Wang June 25, 2020, 3:29 p.m. UTC | #1
On Sun, Jun 21, 2020 at 09:30:12PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> It's old patch, rebase with master, now have tested with the mpeg2 sample
> for ticket 6105, the output srt is same.
> 
>  libavcodec/mpeg12dec.c | 85 ++++++++++++++++++++++++--------------------------
>  1 file changed, 40 insertions(+), 45 deletions(-)
> 
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 1253bde..f0f92ac 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -57,8 +57,7 @@ typedef struct Mpeg1Context {
>      AVPanScan pan_scan;         /* some temporary storage for the panscan */
>      AVStereo3D stereo3d;
>      int has_stereo3d;
> -    uint8_t *a53_caption;
> -    int a53_caption_size;
> +    AVBufferRef *a53_buf_ref;
>      uint8_t afd;
>      int has_afd;
>      int slice_count;
> @@ -1635,13 +1634,13 @@ static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int buf_size)
>              return AVERROR(ENOMEM);
>          memcpy(pan_scan->data, &s1->pan_scan, sizeof(s1->pan_scan));
>  
> -        if (s1->a53_caption) {
> -            AVFrameSideData *sd = av_frame_new_side_data(
> +        if (s1->a53_buf_ref) {
> +            AVFrameSideData *sd = av_frame_new_side_data_from_buf(
>                  s->current_picture_ptr->f, AV_FRAME_DATA_A53_CC,
> -                s1->a53_caption_size);
> -            if (sd)
> -                memcpy(sd->data, s1->a53_caption, s1->a53_caption_size);
> -            av_freep(&s1->a53_caption);
> +                s1->a53_buf_ref);
> +            if (!sd)
> +                av_buffer_unref(&s1->a53_buf_ref);
> +            s1->a53_buf_ref = NULL;
>          }
>  
>          if (s1->has_stereo3d) {
> @@ -2242,26 +2241,18 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
>          /* extract A53 Part 4 CC data */
>          int cc_count = p[5] & 0x1f;
>          if (cc_count > 0 && buf_size >= 7 + cc_count * 3) {
> -            if (s1->a53_caption && s1->a53_caption_size > 0) {
> -                uint8_t *old_a53_caption = s1->a53_caption;
> -                int old_a53_caption_size = s1->a53_caption_size;
> -
> -                s1->a53_caption_size = old_a53_caption_size + cc_count * 3;
> -                s1->a53_caption      = av_malloc(s1->a53_caption_size);
> -                if (s1->a53_caption) {
> -                    memcpy(s1->a53_caption, old_a53_caption, old_a53_caption_size);
> -                    memcpy(s1->a53_caption + old_a53_caption_size, p + 7, cc_count * 3);
> -                }
> -                av_freep(&old_a53_caption);
> -            } else {
> -                s1->a53_caption_size = cc_count * 3;
> -                s1->a53_caption      = av_malloc(s1->a53_caption_size);
> -                if (s1->a53_caption)
> -                    memcpy(s1->a53_caption, p + 7, s1->a53_caption_size);
> -            }
> -            if (!s1->a53_caption) {
> -                s1->a53_caption_size = 0;
> -            }
> +            int old_size = s1->a53_buf_ref ? s1->a53_buf_ref->size : 0;
> +            const uint64_t new_size = (old_size + cc_count
> +                                            * UINT64_C(3));
> +            int ret;
> +
> +            if (new_size > INT_MAX)
> +                return AVERROR(EINVAL);
> +
> +            ret = av_buffer_realloc(&s1->a53_buf_ref, new_size);
> +            if (ret >= 0)
> +                memcpy(s1->a53_buf_ref->data + old_size, p + 7, cc_count * UINT64_C(3));
> +
>              avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
>          }
>          return 1;
> @@ -2270,19 +2261,21 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
>          /* extract SCTE-20 CC data */
>          GetBitContext gb;
>          int cc_count = 0;
> -        int i;
> +        int i, ret;
>  
>          init_get_bits(&gb, p + 2, buf_size - 2);
>          cc_count = get_bits(&gb, 5);
>          if (cc_count > 0) {
> -            av_freep(&s1->a53_caption);
> -            s1->a53_caption_size = cc_count * 3;
> -            s1->a53_caption      = av_mallocz(s1->a53_caption_size);
> -            if (!s1->a53_caption) {
> -                s1->a53_caption_size = 0;
> -            } else {
> +            int old_size = s1->a53_buf_ref ? s1->a53_buf_ref->size : 0;
> +            const uint64_t new_size = (old_size + cc_count
> +                                            * UINT64_C(3));
> +            if (new_size > INT_MAX)
> +                return AVERROR(EINVAL);
> +
> +            ret = av_buffer_realloc(&s1->a53_buf_ref, new_size);
> +            if (ret >= 0) {
>                  uint8_t field, cc1, cc2;
> -                uint8_t *cap = s1->a53_caption;
> +                uint8_t *cap = s1->a53_buf_ref->data;
>                  for (i = 0; i < cc_count && get_bits_left(&gb) >= 26; i++) {
>                      skip_bits(&gb, 2); // priority
>                      field = get_bits(&gb, 2);
> @@ -2334,21 +2327,23 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
>           * on the even field. There also exist DVDs in the wild that encode an odd field count and the
>           * caption_extra_field_added/caption_odd_field_first bits change per packet to allow that. */
>          int cc_count = 0;
> -        int i;
> +        int i, ret;
>          // There is a caption count field in the data, but it is often
>          // incorrect.  So count the number of captions present.
>          for (i = 5; i + 6 <= buf_size && ((p[i] & 0xfe) == 0xfe); i += 6)
>              cc_count++;
>          // Transform the DVD format into A53 Part 4 format
>          if (cc_count > 0) {
> -            av_freep(&s1->a53_caption);
> -            s1->a53_caption_size = cc_count * 6;
> -            s1->a53_caption      = av_malloc(s1->a53_caption_size);
> -            if (!s1->a53_caption) {
> -                s1->a53_caption_size = 0;
> -            } else {
> +            int old_size = s1->a53_buf_ref ? s1->a53_buf_ref->size : 0;
> +            const uint64_t new_size = (old_size + cc_count
> +                                            * UINT64_C(6));
> +            if (new_size > INT_MAX)
> +                return AVERROR(EINVAL);
> +
> +            ret = av_buffer_realloc(&s1->a53_buf_ref, new_size);
> +            if (ret >= 0) {
>                  uint8_t field1 = !!(p[4] & 0x80);
> -                uint8_t *cap = s1->a53_caption;
> +                uint8_t *cap = s1->a53_buf_ref->data;
>                  p += 5;
>                  for (i = 0; i < cc_count; i++) {
>                      cap[0] = (p[0] == 0xff && field1) ? 0xfc : 0xfd;
> @@ -2886,7 +2881,7 @@ static av_cold int mpeg_decode_end(AVCodecContext *avctx)
>      Mpeg1Context *s = avctx->priv_data;
>  
>      ff_mpv_common_end(&s->mpeg_enc_ctx);
> -    av_freep(&s->a53_caption);
> +    av_buffer_unref(&s->a53_buf_ref);
>      return 0;
>  }
>  
> -- 
> 1.8.3.1
> 

ping, will apply tomorrow if no objection.
diff mbox series

Patch

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 1253bde..f0f92ac 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -57,8 +57,7 @@  typedef struct Mpeg1Context {
     AVPanScan pan_scan;         /* some temporary storage for the panscan */
     AVStereo3D stereo3d;
     int has_stereo3d;
-    uint8_t *a53_caption;
-    int a53_caption_size;
+    AVBufferRef *a53_buf_ref;
     uint8_t afd;
     int has_afd;
     int slice_count;
@@ -1635,13 +1634,13 @@  static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int buf_size)
             return AVERROR(ENOMEM);
         memcpy(pan_scan->data, &s1->pan_scan, sizeof(s1->pan_scan));
 
-        if (s1->a53_caption) {
-            AVFrameSideData *sd = av_frame_new_side_data(
+        if (s1->a53_buf_ref) {
+            AVFrameSideData *sd = av_frame_new_side_data_from_buf(
                 s->current_picture_ptr->f, AV_FRAME_DATA_A53_CC,
-                s1->a53_caption_size);
-            if (sd)
-                memcpy(sd->data, s1->a53_caption, s1->a53_caption_size);
-            av_freep(&s1->a53_caption);
+                s1->a53_buf_ref);
+            if (!sd)
+                av_buffer_unref(&s1->a53_buf_ref);
+            s1->a53_buf_ref = NULL;
         }
 
         if (s1->has_stereo3d) {
@@ -2242,26 +2241,18 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
         /* extract A53 Part 4 CC data */
         int cc_count = p[5] & 0x1f;
         if (cc_count > 0 && buf_size >= 7 + cc_count * 3) {
-            if (s1->a53_caption && s1->a53_caption_size > 0) {
-                uint8_t *old_a53_caption = s1->a53_caption;
-                int old_a53_caption_size = s1->a53_caption_size;
-
-                s1->a53_caption_size = old_a53_caption_size + cc_count * 3;
-                s1->a53_caption      = av_malloc(s1->a53_caption_size);
-                if (s1->a53_caption) {
-                    memcpy(s1->a53_caption, old_a53_caption, old_a53_caption_size);
-                    memcpy(s1->a53_caption + old_a53_caption_size, p + 7, cc_count * 3);
-                }
-                av_freep(&old_a53_caption);
-            } else {
-                s1->a53_caption_size = cc_count * 3;
-                s1->a53_caption      = av_malloc(s1->a53_caption_size);
-                if (s1->a53_caption)
-                    memcpy(s1->a53_caption, p + 7, s1->a53_caption_size);
-            }
-            if (!s1->a53_caption) {
-                s1->a53_caption_size = 0;
-            }
+            int old_size = s1->a53_buf_ref ? s1->a53_buf_ref->size : 0;
+            const uint64_t new_size = (old_size + cc_count
+                                            * UINT64_C(3));
+            int ret;
+
+            if (new_size > INT_MAX)
+                return AVERROR(EINVAL);
+
+            ret = av_buffer_realloc(&s1->a53_buf_ref, new_size);
+            if (ret >= 0)
+                memcpy(s1->a53_buf_ref->data + old_size, p + 7, cc_count * UINT64_C(3));
+
             avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
         }
         return 1;
@@ -2270,19 +2261,21 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
         /* extract SCTE-20 CC data */
         GetBitContext gb;
         int cc_count = 0;
-        int i;
+        int i, ret;
 
         init_get_bits(&gb, p + 2, buf_size - 2);
         cc_count = get_bits(&gb, 5);
         if (cc_count > 0) {
-            av_freep(&s1->a53_caption);
-            s1->a53_caption_size = cc_count * 3;
-            s1->a53_caption      = av_mallocz(s1->a53_caption_size);
-            if (!s1->a53_caption) {
-                s1->a53_caption_size = 0;
-            } else {
+            int old_size = s1->a53_buf_ref ? s1->a53_buf_ref->size : 0;
+            const uint64_t new_size = (old_size + cc_count
+                                            * UINT64_C(3));
+            if (new_size > INT_MAX)
+                return AVERROR(EINVAL);
+
+            ret = av_buffer_realloc(&s1->a53_buf_ref, new_size);
+            if (ret >= 0) {
                 uint8_t field, cc1, cc2;
-                uint8_t *cap = s1->a53_caption;
+                uint8_t *cap = s1->a53_buf_ref->data;
                 for (i = 0; i < cc_count && get_bits_left(&gb) >= 26; i++) {
                     skip_bits(&gb, 2); // priority
                     field = get_bits(&gb, 2);
@@ -2334,21 +2327,23 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
          * on the even field. There also exist DVDs in the wild that encode an odd field count and the
          * caption_extra_field_added/caption_odd_field_first bits change per packet to allow that. */
         int cc_count = 0;
-        int i;
+        int i, ret;
         // There is a caption count field in the data, but it is often
         // incorrect.  So count the number of captions present.
         for (i = 5; i + 6 <= buf_size && ((p[i] & 0xfe) == 0xfe); i += 6)
             cc_count++;
         // Transform the DVD format into A53 Part 4 format
         if (cc_count > 0) {
-            av_freep(&s1->a53_caption);
-            s1->a53_caption_size = cc_count * 6;
-            s1->a53_caption      = av_malloc(s1->a53_caption_size);
-            if (!s1->a53_caption) {
-                s1->a53_caption_size = 0;
-            } else {
+            int old_size = s1->a53_buf_ref ? s1->a53_buf_ref->size : 0;
+            const uint64_t new_size = (old_size + cc_count
+                                            * UINT64_C(6));
+            if (new_size > INT_MAX)
+                return AVERROR(EINVAL);
+
+            ret = av_buffer_realloc(&s1->a53_buf_ref, new_size);
+            if (ret >= 0) {
                 uint8_t field1 = !!(p[4] & 0x80);
-                uint8_t *cap = s1->a53_caption;
+                uint8_t *cap = s1->a53_buf_ref->data;
                 p += 5;
                 for (i = 0; i < cc_count; i++) {
                     cap[0] = (p[0] == 0xff && field1) ? 0xfc : 0xfd;
@@ -2886,7 +2881,7 @@  static av_cold int mpeg_decode_end(AVCodecContext *avctx)
     Mpeg1Context *s = avctx->priv_data;
 
     ff_mpv_common_end(&s->mpeg_enc_ctx);
-    av_freep(&s->a53_caption);
+    av_buffer_unref(&s->a53_buf_ref);
     return 0;
 }