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

[James Almer]

On 12/19/2019 2:09 AM, lance.lmwang at gmail.com wrote:
> From: Limin Wang <lance.lmwang at gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> ---
>  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(&s->unregistered, gb, size);
>      case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
>          return decode_nal_sei_alternative_transfer(&s->alternative_transfer, gb);
>      default:
> @@ -365,4 +394,9 @@ void ff_hevc_reset_sei(HEVCSEI *s)
>  {
>      s->a53_caption.a53_caption_size = 0;
>      av_freep(&s->a53_caption.a53_caption);
> +
> +    for (int i = 0; i < s->unregistered.nb_buf_ref; i++)
> +        av_buffer_unref(&s->unregistered.buf_ref[i]);
> +    s->unregistered.nb_buf_ref = 0;
> +    av_freep(&s->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 = &s->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(&unreg->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(&s->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 Unregistered
> +[/SIDE_DATA]
>  [/FRAME]
>  [STREAM]
>  width=384
>