[FFmpeg-devel] [PATCH v5 4/4] avcodec/h264: create user data unregistered side data H.264

[Limin Wang]

On Mon, Jan 06, 2020 at 10:18:13PM +0000, Mark Thompson wrote:
> On 02/01/2020 01:28, 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/h264_sei.c     |  20 +++--
> >  libavcodec/h264_sei.h     |   2 +
> >  libavcodec/h264_slice.c   |  14 ++++
> >  tests/ref/fate/mov-zombie | 195 ++++++++++++++++++++++++++++++----------------
> >  4 files changed, 161 insertions(+), 70 deletions(-)
> > 
> > diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> > index a565fea..43e2814 100644
> > --- a/libavcodec/h264_sei.c
> > +++ b/libavcodec/h264_sei.c
> > @@ -52,6 +52,10 @@ void ff_h264_sei_uninit(H264SEIContext *h)
> >      h->afd.present                 =  0;
> >  
> >      av_buffer_unref(&h->a53_caption.buf_ref);
> > +    for (int i = 0; i < h->unregistered.nb_buf_ref; i++)
> > +        av_buffer_unref(&h->unregistered.buf_ref[i]);
> > +    h->unregistered.nb_buf_ref = 0;
> > +    av_freep(&h->unregistered.buf_ref);
> >  }
> >  
> >  static int decode_picture_timing(H264SEIPictureTiming *h, GetBitContext *gb,
> > @@ -246,25 +250,31 @@ static int decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
> >  {
> >      uint8_t *user_data;
> >      int e, build, i;
> > +    AVBufferRef *buf_ref, **tmp;
> >  
> > -    if (size < 16 || size >= INT_MAX - 1)
> > +    if (size < 16)
> >          return AVERROR_INVALIDDATA;
> >  
> > -    user_data = av_malloc(size + 1);
> > -    if (!user_data)
> > +    tmp = av_realloc_array(h->buf_ref, h->nb_buf_ref + 1, sizeof(*h->buf_ref));
> > +    if (!tmp)
> >          return AVERROR(ENOMEM);
> > +    h->buf_ref = tmp;
> > +
> > +    buf_ref = av_buffer_alloc(size);
> > +    if (!buf_ref)
> > +        return AVERROR(ENOMEM);> +    user_data = buf_ref->data;
> >  
> >      for (i = 0; i < size; i++)
> >          user_data[i] = get_bits(gb, 8);
> > +    h->buf_ref[h->nb_buf_ref++] = buf_ref;
> >  
> > -    user_data[i] = 0;
> 
> You've lost this terminator, which allows

I need keep the user data same with original data so remove the extra terminator
data, I prefer to add a isprint() checking before x264 string processing.

> 
> >      e = sscanf(user_data + 16, "x264 - core %d", &build);
> 
> to read over the end of the allocated buffer.
> 
> >      if (e == 1 && build > 0)
> >          h->x264_build = build;
> >      if (e == 1 && build == 1 && !strncmp(user_data+16, "x264 - core 0000", 16))
> >          h->x264_build = 67;
> >  
> > -    av_free(user_data);
> >      return 0;
> >  }
> >  
> > diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
> > index a75c3aa..aa4595f 100644
> > --- a/libavcodec/h264_sei.h
> > +++ b/libavcodec/h264_sei.h
> > @@ -121,6 +121,8 @@ typedef struct H264SEIA53Caption {
> >  
> >  typedef struct H264SEIUnregistered {
> >      int x264_build;
> > +    AVBufferRef **buf_ref;
> > +    int nb_buf_ref;
> >  } H264SEIUnregistered;
> >  
> >  typedef struct H264SEIRecoveryPoint {
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index e24d41c..ea967c8 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1285,6 +1285,20 @@ static int h264_export_frame_props(H264Context *h)
> >          h->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> >      }
> >  
> > +    for (int i = 0; i < h->sei.unregistered.nb_buf_ref; i++) {
> > +        H264SEIUnregistered *unreg = &h->sei.unregistered;
> > +
> > +        if (unreg->buf_ref[i]) {
> > +            AVFrameSideData *sd = av_frame_new_side_data_from_buf(cur->f,
> > +                    AV_FRAME_DATA_USER_DATA_UNREGISTERED,
> > +                    unreg->buf_ref[i]);
> > +            if (!sd)
> > +                av_buffer_unref(&unreg->buf_ref[i]);
> > +            unreg->buf_ref[i] = NULL;
> > +        }
> > +    }
> > +    h->sei.unregistered.nb_buf_ref = 0;
> > +
> >      if (h->sei.picture_timing.timecode_cnt > 0) {
> >          uint32_t tc = 0;
> >          uint32_t *tc_sd;
> 
> Everything else looks good to me.
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang