Re: [FFmpeg-devel] [PATCH v3 2/5] avcodec/hevc_sei: add support for user data unregistered SEI message

2019-12-20 Thread Limin Wang
On Fri, Dec 20, 2019 at 02:23:40PM -0300, James Almer wrote:
> On 12/19/2019 2:09 AM, lance.lmw...@gmail.com wrote:
> > From: Limin Wang 
> > 
> > Signed-off-by: Limin Wang 
> > ---
> >  libavcodec/hevc_sei.c   | 34 +
> >  libavcodec/hevc_sei.h   |  6 +
> >  libavcodec/hevcdec.c| 18 +++
> >  tests/ref/fate/hevc-monochrome-crop |  3 +++
> >  4 files changed, 61 insertions(+)
> > 
> > diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> > index c59bd4321e..7f738a049c 100644
> > --- a/libavcodec/hevc_sei.c
> > +++ b/libavcodec/hevc_sei.c
> > @@ -206,6 +206,33 @@ static int 
> > decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
> >  return 0;
> >  }
> >  
> > +static int decode_nal_sei_user_data_unregistered(HEVCSEIUnregistered *s, 
> > GetBitContext *gb,
> > +  int size)
> > +{
> > +AVBufferRef *buf_ref, **tmp;
> > +
> > +if (size < 16)
> > +   return AVERROR(EINVAL);
> > +
> > +if (s->nb_buf_ref > INT_MAX / sizeof(*s->buf_ref) - 1)
> > +   return AVERROR(EINVAL);
> > +
> > +tmp = av_realloc(s->buf_ref, (s->nb_buf_ref + 1) * 
> > sizeof(*s->buf_ref));
> 
> Use av_realloc_array() instead and you can remove the above check.

will update to use it


> 
> > +if (!tmp)
> > +return AVERROR(ENOMEM);
> > +s->buf_ref = tmp;
> > +
> > +buf_ref = av_buffer_alloc(size);
> > +if (!buf_ref)
> > +return AVERROR(ENOMEM);
> > +
> > +for (int i = 0; i < size; i++)
> > +buf_ref->data[i] = get_bits(gb, 8);
> > +s->buf_ref[s->nb_buf_ref++] = buf_ref;
> > +
> > +return 0;
> > +}
> > +
> >  static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, 
> > GetBitContext *gb,
> >   int size)
> >  {
> > @@ -293,6 +320,8 @@ static int decode_nal_sei_prefix(GetBitContext *gb, 
> > void *logctx, HEVCSEI *s,
> >  return decode_nal_sei_active_parameter_sets(s, gb, logctx);
> >  case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
> >  return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
> > +case HEVC_SEI_TYPE_USER_DATA_UNREGISTERED:
> > +return decode_nal_sei_user_data_unregistered(>unregistered, gb, 
> > size);
> >  case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
> >  return 
> > decode_nal_sei_alternative_transfer(>alternative_transfer, gb);
> >  default:
> > @@ -365,4 +394,9 @@ void ff_hevc_reset_sei(HEVCSEI *s)
> >  {
> >  s->a53_caption.a53_caption_size = 0;
> >  av_freep(>a53_caption.a53_caption);
> > +
> > +for (int i = 0; i < s->unregistered.nb_buf_ref; i++)
> > +av_buffer_unref(>unregistered.buf_ref[i]);
> > +s->unregistered.nb_buf_ref = 0;
> > +av_freep(>unregistered.buf_ref);
> >  }
> > diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> > index f6516ae982..2464117d47 100644
> > --- a/libavcodec/hevc_sei.h
> > +++ b/libavcodec/hevc_sei.h
> > @@ -87,6 +87,11 @@ typedef struct HEVCSEIA53Caption {
> >  uint8_t *a53_caption;
> >  } HEVCSEIA53Caption;
> >  
> > +typedef struct HEVCSEIUnregistered {
> > +AVBufferRef **buf_ref;
> > +int nb_buf_ref;
> > +} HEVCSEIUnregistered;
> > +
> >  typedef struct HEVCSEIMasteringDisplay {
> >  int present;
> >  uint16_t display_primaries[3][2];
> > @@ -112,6 +117,7 @@ typedef struct HEVCSEI {
> >  HEVCSEIDisplayOrientation display_orientation;
> >  HEVCSEIPictureTiming picture_timing;
> >  HEVCSEIA53Caption a53_caption;
> > +HEVCSEIUnregistered unregistered;
> >  HEVCSEIMasteringDisplay mastering_display;
> >  HEVCSEIContentLight content_light;
> >  int active_seq_parameter_set_id;
> > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > index 8f1c162ace..a4981e983c 100644
> > --- a/libavcodec/hevcdec.c
> > +++ b/libavcodec/hevcdec.c
> > @@ -2789,6 +2789,24 @@ static int set_side_data(HEVCContext *s)
> >  s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> >  }
> >  
> > +for (int i = 0; i < s->sei.unregistered.nb_buf_ref; i++) {
> > +HEVCSEIUnregistered *unreg = >sei.unregistered;
> > +
> > +if (unreg->buf_ref[i]) {
> > +AVFrameSideData *sd = av_frame_new_side_data_from_buf(out,
> > +AV_FRAME_DATA_USER_DATA_UNREGISTERED,
> > +unreg->buf_ref[i]);
> > +if (!sd)
> > +av_buffer_unref(>buf_ref[i]);
> > +unreg->buf_ref[i] = NULL;
> > +}
> > +}
> > +
> > +if (s->sei.unregistered.nb_buf_ref > 0) {
> > +s->sei.unregistered.nb_buf_ref = 0;
> > +av_freep(>sei.unregistered.buf_ref);
> 
> No need to free the array since it can be reused. And if it's never
> reused, it will be freed by ff_hevc_reset_sei() anyway.
> 
> Just set s->sei.unregistered.nb_buf_ref to 0 unconditionally.

yes, 

Re: [FFmpeg-devel] [PATCH v3 2/5] avcodec/hevc_sei: add support for user data unregistered SEI message

2019-12-20 Thread James Almer
On 12/19/2019 2:09 AM, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  libavcodec/hevc_sei.c   | 34 +
>  libavcodec/hevc_sei.h   |  6 +
>  libavcodec/hevcdec.c| 18 +++
>  tests/ref/fate/hevc-monochrome-crop |  3 +++
>  4 files changed, 61 insertions(+)
> 
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index c59bd4321e..7f738a049c 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -206,6 +206,33 @@ static int 
> decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
>  return 0;
>  }
>  
> +static int decode_nal_sei_user_data_unregistered(HEVCSEIUnregistered *s, 
> GetBitContext *gb,
> +  int size)
> +{
> +AVBufferRef *buf_ref, **tmp;
> +
> +if (size < 16)
> +   return AVERROR(EINVAL);
> +
> +if (s->nb_buf_ref > INT_MAX / sizeof(*s->buf_ref) - 1)
> +   return AVERROR(EINVAL);
> +
> +tmp = av_realloc(s->buf_ref, (s->nb_buf_ref + 1) * sizeof(*s->buf_ref));

Use av_realloc_array() instead and you can remove the above check.

> +if (!tmp)
> +return AVERROR(ENOMEM);
> +s->buf_ref = tmp;
> +
> +buf_ref = av_buffer_alloc(size);
> +if (!buf_ref)
> +return AVERROR(ENOMEM);
> +
> +for (int i = 0; i < size; i++)
> +buf_ref->data[i] = get_bits(gb, 8);
> +s->buf_ref[s->nb_buf_ref++] = buf_ref;
> +
> +return 0;
> +}
> +
>  static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, 
> GetBitContext *gb,
>   int size)
>  {
> @@ -293,6 +320,8 @@ static int decode_nal_sei_prefix(GetBitContext *gb, void 
> *logctx, HEVCSEI *s,
>  return decode_nal_sei_active_parameter_sets(s, gb, logctx);
>  case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
>  return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
> +case HEVC_SEI_TYPE_USER_DATA_UNREGISTERED:
> +return decode_nal_sei_user_data_unregistered(>unregistered, gb, 
> size);
>  case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
>  return decode_nal_sei_alternative_transfer(>alternative_transfer, 
> gb);
>  default:
> @@ -365,4 +394,9 @@ void ff_hevc_reset_sei(HEVCSEI *s)
>  {
>  s->a53_caption.a53_caption_size = 0;
>  av_freep(>a53_caption.a53_caption);
> +
> +for (int i = 0; i < s->unregistered.nb_buf_ref; i++)
> +av_buffer_unref(>unregistered.buf_ref[i]);
> +s->unregistered.nb_buf_ref = 0;
> +av_freep(>unregistered.buf_ref);
>  }
> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> index f6516ae982..2464117d47 100644
> --- a/libavcodec/hevc_sei.h
> +++ b/libavcodec/hevc_sei.h
> @@ -87,6 +87,11 @@ typedef struct HEVCSEIA53Caption {
>  uint8_t *a53_caption;
>  } HEVCSEIA53Caption;
>  
> +typedef struct HEVCSEIUnregistered {
> +AVBufferRef **buf_ref;
> +int nb_buf_ref;
> +} HEVCSEIUnregistered;
> +
>  typedef struct HEVCSEIMasteringDisplay {
>  int present;
>  uint16_t display_primaries[3][2];
> @@ -112,6 +117,7 @@ typedef struct HEVCSEI {
>  HEVCSEIDisplayOrientation display_orientation;
>  HEVCSEIPictureTiming picture_timing;
>  HEVCSEIA53Caption a53_caption;
> +HEVCSEIUnregistered unregistered;
>  HEVCSEIMasteringDisplay mastering_display;
>  HEVCSEIContentLight content_light;
>  int active_seq_parameter_set_id;
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 8f1c162ace..a4981e983c 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -2789,6 +2789,24 @@ static int set_side_data(HEVCContext *s)
>  s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
>  }
>  
> +for (int i = 0; i < s->sei.unregistered.nb_buf_ref; i++) {
> +HEVCSEIUnregistered *unreg = >sei.unregistered;
> +
> +if (unreg->buf_ref[i]) {
> +AVFrameSideData *sd = av_frame_new_side_data_from_buf(out,
> +AV_FRAME_DATA_USER_DATA_UNREGISTERED,
> +unreg->buf_ref[i]);
> +if (!sd)
> +av_buffer_unref(>buf_ref[i]);
> +unreg->buf_ref[i] = NULL;
> +}
> +}
> +
> +if (s->sei.unregistered.nb_buf_ref > 0) {
> +s->sei.unregistered.nb_buf_ref = 0;
> +av_freep(>sei.unregistered.buf_ref);

No need to free the array since it can be reused. And if it's never
reused, it will be freed by ff_hevc_reset_sei() anyway.

Just set s->sei.unregistered.nb_buf_ref to 0 unconditionally.

> +}
> +
>  return 0;
>  }
>  
> diff --git a/tests/ref/fate/hevc-monochrome-crop 
> b/tests/ref/fate/hevc-monochrome-crop
> index 4e45412acf..43f0abbcfa 100644
> --- a/tests/ref/fate/hevc-monochrome-crop
> +++ b/tests/ref/fate/hevc-monochrome-crop
> @@ -1,6 +1,9 @@
>  [FRAME]
>  width=384
>  height=240
> +[SIDE_DATA]
> +side_data_type=User Data 

[FFmpeg-devel] [PATCH v3 2/5] avcodec/hevc_sei: add support for user data unregistered SEI message

2019-12-18 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavcodec/hevc_sei.c   | 34 +
 libavcodec/hevc_sei.h   |  6 +
 libavcodec/hevcdec.c| 18 +++
 tests/ref/fate/hevc-monochrome-crop |  3 +++
 4 files changed, 61 insertions(+)

diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
index c59bd4321e..7f738a049c 100644
--- a/libavcodec/hevc_sei.c
+++ b/libavcodec/hevc_sei.c
@@ -206,6 +206,33 @@ static int 
decode_registered_user_data_closed_caption(HEVCSEIA53Caption *s, GetB
 return 0;
 }
 
+static int decode_nal_sei_user_data_unregistered(HEVCSEIUnregistered *s, 
GetBitContext *gb,
+  int size)
+{
+AVBufferRef *buf_ref, **tmp;
+
+if (size < 16)
+   return AVERROR(EINVAL);
+
+if (s->nb_buf_ref > INT_MAX / sizeof(*s->buf_ref) - 1)
+   return AVERROR(EINVAL);
+
+tmp = av_realloc(s->buf_ref, (s->nb_buf_ref + 1) * sizeof(*s->buf_ref));
+if (!tmp)
+return AVERROR(ENOMEM);
+s->buf_ref = tmp;
+
+buf_ref = av_buffer_alloc(size);
+if (!buf_ref)
+return AVERROR(ENOMEM);
+
+for (int i = 0; i < size; i++)
+buf_ref->data[i] = get_bits(gb, 8);
+s->buf_ref[s->nb_buf_ref++] = buf_ref;
+
+return 0;
+}
+
 static int decode_nal_sei_user_data_registered_itu_t_t35(HEVCSEI *s, 
GetBitContext *gb,
  int size)
 {
@@ -293,6 +320,8 @@ static int decode_nal_sei_prefix(GetBitContext *gb, void 
*logctx, HEVCSEI *s,
 return decode_nal_sei_active_parameter_sets(s, gb, logctx);
 case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
 return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
+case HEVC_SEI_TYPE_USER_DATA_UNREGISTERED:
+return decode_nal_sei_user_data_unregistered(>unregistered, gb, 
size);
 case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
 return decode_nal_sei_alternative_transfer(>alternative_transfer, 
gb);
 default:
@@ -365,4 +394,9 @@ void ff_hevc_reset_sei(HEVCSEI *s)
 {
 s->a53_caption.a53_caption_size = 0;
 av_freep(>a53_caption.a53_caption);
+
+for (int i = 0; i < s->unregistered.nb_buf_ref; i++)
+av_buffer_unref(>unregistered.buf_ref[i]);
+s->unregistered.nb_buf_ref = 0;
+av_freep(>unregistered.buf_ref);
 }
diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
index f6516ae982..2464117d47 100644
--- a/libavcodec/hevc_sei.h
+++ b/libavcodec/hevc_sei.h
@@ -87,6 +87,11 @@ typedef struct HEVCSEIA53Caption {
 uint8_t *a53_caption;
 } HEVCSEIA53Caption;
 
+typedef struct HEVCSEIUnregistered {
+AVBufferRef **buf_ref;
+int nb_buf_ref;
+} HEVCSEIUnregistered;
+
 typedef struct HEVCSEIMasteringDisplay {
 int present;
 uint16_t display_primaries[3][2];
@@ -112,6 +117,7 @@ typedef struct HEVCSEI {
 HEVCSEIDisplayOrientation display_orientation;
 HEVCSEIPictureTiming picture_timing;
 HEVCSEIA53Caption a53_caption;
+HEVCSEIUnregistered unregistered;
 HEVCSEIMasteringDisplay mastering_display;
 HEVCSEIContentLight content_light;
 int active_seq_parameter_set_id;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 8f1c162ace..a4981e983c 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2789,6 +2789,24 @@ static int set_side_data(HEVCContext *s)
 s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
 }
 
+for (int i = 0; i < s->sei.unregistered.nb_buf_ref; i++) {
+HEVCSEIUnregistered *unreg = >sei.unregistered;
+
+if (unreg->buf_ref[i]) {
+AVFrameSideData *sd = av_frame_new_side_data_from_buf(out,
+AV_FRAME_DATA_USER_DATA_UNREGISTERED,
+unreg->buf_ref[i]);
+if (!sd)
+av_buffer_unref(>buf_ref[i]);
+unreg->buf_ref[i] = NULL;
+}
+}
+
+if (s->sei.unregistered.nb_buf_ref > 0) {
+s->sei.unregistered.nb_buf_ref = 0;
+av_freep(>sei.unregistered.buf_ref);
+}
+
 return 0;
 }
 
diff --git a/tests/ref/fate/hevc-monochrome-crop 
b/tests/ref/fate/hevc-monochrome-crop
index 4e45412acf..43f0abbcfa 100644
--- a/tests/ref/fate/hevc-monochrome-crop
+++ b/tests/ref/fate/hevc-monochrome-crop
@@ -1,6 +1,9 @@
 [FRAME]
 width=384
 height=240
+[SIDE_DATA]
+side_data_type=User Data Unregistered
+[/SIDE_DATA]
 [/FRAME]
 [STREAM]
 width=384
-- 
2.21.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-requ...@ffmpeg.org with subject "unsubscribe".