On Sun, 09 Jul 2017 22:59:41 +0000
Kieran Kunhya <kier...@obe.tv> wrote:
> From e8768677511ae5ae9c62c7182a73993e5132f5b8 Mon Sep 17 00:00:00 2001
> From: Kieran Kunhya <kier...@obe.tv>
> Date: Sun, 9 Jul 2017 23:56:14 +0100
> Subject: [PATCH] h264: Support multi-field closed captions by using
> AVBufferRef and not resetting per field
>
> ---
> libavcodec/h264_sei.c | 15 ++++++++-------
> libavcodec/h264_sei.h | 3 +--
> libavcodec/h264_slice.c | 15 ++++++++++-----
> libavcodec/h264dec.c | 5 +++--
> 4 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index a7e627e..3dcac7e 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -51,8 +51,7 @@ void ff_h264_sei_uninit(H264SEIContext *h)
> h->display_orientation.present = 0;
> h->afd.present = 0;
>
> - h->a53_caption.a53_caption_size = 0;
> - av_freep(&h->a53_caption.a53_caption);
> + av_buffer_unref(&h->a53_caption.buf_ref);
> }
>
> static int decode_picture_timing(H264SEIPictureTiming *h, GetBitContext *gb,
> @@ -169,7 +168,8 @@ static int
> decode_registered_user_data_closed_caption(H264SEIA53Caption *h,
> size -= 2;
>
> if (cc_count && size >= cc_count * 3) {
> - const uint64_t new_size = (h->a53_caption_size + cc_count
> + int old_size = h->buf_ref ? h->buf_ref->size : 0;
> + const uint64_t new_size = (old_size + cc_count
> * UINT64_C(3));
> int i, ret;
>
> @@ -177,14 +177,15 @@ static int
> decode_registered_user_data_closed_caption(H264SEIA53Caption *h,
> return AVERROR(EINVAL);
>
> /* Allow merging of the cc data from two fields. */
> - ret = av_reallocp(&h->a53_caption, new_size);
> + ret = av_buffer_realloc(&h->buf_ref, new_size);
> if (ret < 0)
> return ret;
>
> + /* Use of av_buffer_realloc assumes buffer is writeable */
> for (i = 0; i < cc_count; i++) {
> - h->a53_caption[h->a53_caption_size++] = get_bits(gb, 8);
> - h->a53_caption[h->a53_caption_size++] = get_bits(gb, 8);
> - h->a53_caption[h->a53_caption_size++] = get_bits(gb, 8);
> + h->buf_ref->data[old_size++] = get_bits(gb, 8);
> + h->buf_ref->data[old_size++] = get_bits(gb, 8);
> + h->buf_ref->data[old_size++] = get_bits(gb, 8);
> }
>
> skip_bits(gb, 8); // marker_bits
> diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
> index da3b391..7d092e4 100644
> --- a/libavcodec/h264_sei.h
> +++ b/libavcodec/h264_sei.h
> @@ -91,8 +91,7 @@ typedef struct H264SEIAFD {
> } H264SEIAFD;
>
> typedef struct H264SEIA53Caption {
> - int a53_caption_size;
> - uint8_t *a53_caption;
> + AVBufferRef *buf_ref;
> } H264SEIA53Caption;
>
> typedef struct H264SEIUnregistered {
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index 6deb08f..39662a6 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -430,6 +430,12 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
> MAX_DELAYED_PIC_COUNT + 2, h, h1);
>
> h->frame_recovered = h1->frame_recovered;
> + if (h1->sei.a53_caption.buf_ref) {
> + h->sei.a53_caption.buf_ref =
> av_buffer_ref(h1->sei.a53_caption.buf_ref);
> + av_buffer_unref(&h1->sei.a53_caption.buf_ref);
> + }
> + else
> + h->sei.a53_caption.buf_ref = NULL;
That seems overly convoluted. Since you only want to move the
reference instead of creating a new one, you could just assign the
AVBufferRef pointer and assign the source field to NULL.
>
> if (!h->cur_pic_ptr)
> return 0;
> @@ -1276,15 +1282,14 @@ static int h264_export_frame_props(H264Context *h)
> }
> }
>
> - if (h->sei.a53_caption.a53_caption) {
> + if (h->sei.a53_caption.buf_ref) {
> H264SEIA53Caption *a53 = &h->sei.a53_caption;
> AVFrameSideData *sd = av_frame_new_side_data(cur->f,
> AV_FRAME_DATA_A53_CC,
> - a53->a53_caption_size);
> + a53->buf_ref->size);
> if (sd)
> - memcpy(sd->data, a53->a53_caption, a53->a53_caption_size);
> - av_freep(&a53->a53_caption);
> - a53->a53_caption_size = 0;
> + memcpy(sd->data, a53->buf_ref->data, a53->buf_ref->size);
> + av_buffer_unref(&a53->buf_ref);
(In theory you could avoid the memcpy and set the buffer ref on the
side data, but that would probably cause more trouble than it solves.)
> h->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> }
>
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 49ebeca..4111838 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -613,9 +613,10 @@ static int decode_nal_units(H264Context *h, const
> uint8_t *buf, int buf_size)
>
> if (!(avctx->flags2 & AV_CODEC_FLAG2_CHUNKS)) {
> h->current_slice = 0;
> - if (!h->first_field)
> + if (!h->first_field) {
> h->cur_pic_ptr = NULL;
> - ff_h264_sei_uninit(&h->sei);
> + ff_h264_sei_uninit(&h->sei);
> + }
Seems slightly dangerous, since it could potentially affect other
things than CCs, but probably ok.
> }
>
> if (h->nal_length_size == 4) {
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
