diff mbox

[FFmpeg-devel] lavc/videotoolboxenc: implement a53cc

Message ID 1473322783-39263-1-git-send-email-ffmpeg@tmm1.net
State Accepted
Headers show

Commit Message

Aman Gupta Sept. 8, 2016, 8:19 a.m. UTC
From: Aman Gupta <aman@tmm1.net>

---
 libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 9 deletions(-)

Comments

Carl Eugen Hoyos Sept. 8, 2016, 1:28 p.m. UTC | #1
2016-09-08 10:19 GMT+02:00 Aman Gupta <ffmpeg@tmm1.net>:

> +    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },

Why is this disabled by default?

Carl Eugen
Aman Gupta Sept. 8, 2016, 1:48 p.m. UTC | #2
On Thursday, September 8, 2016, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2016-09-08 10:19 GMT+02:00 Aman Gupta <ffmpeg@tmm1.net <javascript:;>>:
>
> > +    { "a53cc", "Use A53 Closed Captions (if available)",
> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>
> Why is this disabled by default?


I copied this from the libx264 and qsv encoders, which also disable by
default. Not sure why.


>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <javascript:;>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Carl Eugen Hoyos Sept. 8, 2016, 4:18 p.m. UTC | #3
2016-09-08 15:48 GMT+02:00 Aman Gupta <themastermind1@gmail.com>:
> On Thursday, September 8, 2016, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
>> 2016-09-08 10:19 GMT+02:00 Aman Gupta <ffmpeg@tmm1.net <javascript:;>>:
>>
>> > +    { "a53cc", "Use A53 Closed Captions (if available)",
>> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>>
>> Why is this disabled by default?
>
> I copied this from the libx264 and qsv encoders, which also disable by
> default. Not sure why.

If you believe it should be enabled by default, please send a patch that
enables the function by default.
(If there is no issue, the others can be fixed.)

Thank you, Carl Eugen
Aman Gupta Sept. 9, 2016, 1:50 a.m. UTC | #4
On Thu, Sep 8, 2016 at 9:18 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2016-09-08 15:48 GMT+02:00 Aman Gupta <themastermind1@gmail.com>:
> > On Thursday, September 8, 2016, Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
> >
> >> 2016-09-08 10:19 GMT+02:00 Aman Gupta <ffmpeg@tmm1.net <javascript:;>>:
> >>
> >> > +    { "a53cc", "Use A53 Closed Captions (if available)",
> >> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
> >>
> >> Why is this disabled by default?
> >
> > I copied this from the libx264 and qsv encoders, which also disable by
> > default. Not sure why.
>
> If you believe it should be enabled by default, please send a patch that
> enables the function by default.
> (If there is no issue, the others can be fixed.)
>

I don't have a good enough understanding of the possible consequences to
feel comfortable enabling it by default yet.


>
> Thank you, Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Rick Kern Sept. 10, 2016, 6:07 p.m. UTC | #5
> On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> 
> From: Aman Gupta <aman@tmm1.net>
> 
> ---
> libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 4345ca3..859dde9 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -32,6 +32,7 @@
> #include "libavutil/pixdesc.h"
> #include "internal.h"
> #include <pthread.h>
> +#include "h264.h"
> 
> #if !CONFIG_VT_BT2020
> # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
> @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
> 
> static const uint8_t start_code[] = { 0, 0, 0, 1 };
> 
> +typedef struct ExtraSEI {
> +  void *data;
> +  size_t size;
> +} ExtraSEI;
> +
> typedef struct BufNode {
>     CMSampleBufferRef cm_buffer;
> +    ExtraSEI *sei;
>     struct BufNode* next;
>     int error;
> } BufNode;
> @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>     bool flushing;
>     bool has_b_frames;
>     bool warned_color_range;
> +    bool a53_cc;
> } VTEncContext;
> 
> static int vtenc_populate_extradata(AVCodecContext   *avctx,
> @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx, int err)
>     pthread_mutex_unlock(&vtctx->lock);
> }
> 
> -static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
> +static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, ExtraSEI **sei)
> {
>     BufNode *info;
> 
> @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>     pthread_mutex_unlock(&vtctx->lock);
> 
>     *buf = info->cm_buffer;
> +    if (sei && *buf) {
> +        *sei = info->sei;
> +    } else if (info->sei) {
> +        if (info->sei->data) av_free(info->sei->data);
> +        av_free(info->sei);
> +    }
>     av_free(info);
> 
>     vtctx->frame_ct_out++;
> @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>     return 0;
> }
> 
> -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
> +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer, ExtraSEI *sei)
> {
>     BufNode *info = av_malloc(sizeof(BufNode));
>     if (!info) {
> @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
> 
>     CFRetain(buffer);
>     info->cm_buffer = buffer;
> +    info->sei = sei;
>     info->next = NULL;
> 
>     pthread_mutex_lock(&vtctx->lock);
> @@ -420,6 +435,7 @@ static void vtenc_output_callback(
> {
>     AVCodecContext *avctx = ctx;
>     VTEncContext   *vtctx = avctx->priv_data;
> +    ExtraSEI *sei = sourceFrameCtx;
> 
>     if (vtctx->async_error) {
>         if(sample_buffer) CFRelease(sample_buffer);
> @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>         }
>     }
> 
> -    vtenc_q_push(vtctx, sample_buffer);
> +    vtenc_q_push(vtctx, sample_buffer, sei);
> }
> 
> static int get_length_code_size(
> @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
> static int vtenc_cm_to_avpacket(
>     AVCodecContext    *avctx,
>     CMSampleBufferRef sample_buffer,
> -    AVPacket          *pkt)
> +    AVPacket          *pkt,
> +    ExtraSEI          *sei)
> {
>     VTEncContext *vtctx = avctx->priv_data;
> 
> @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>     size_t  header_size = 0;
>     size_t  in_buf_size;
>     size_t  out_buf_size;
> +    size_t  sei_nalu_size = 0;
>     int64_t dts_delta;
>     int64_t time_base_num;
>     int nalu_count;
> @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>     if(status)
>         return status;
> 
> +    if (sei) {
> +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
> +    }
> +
>     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>     out_buf_size = header_size +
>                    in_buf_size +
> +                   sei_nalu_size +
>                    nalu_count * ((int)sizeof(start_code) - (int)length_code_size);
> 
>     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
> @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>         length_code_size,
>         sample_buffer,
>         pkt->data + header_size,
> -        pkt->size - header_size
> +        pkt->size - header_size - sei_nalu_size
>     );
> 
>     if (status) {
> @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>         return status;
>     }
> 
> +    if (sei_nalu_size > 0) {
> +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
> +        memcpy(sei_nalu, start_code, sizeof(start_code));
> +        sei_nalu += sizeof(start_code);
> +        sei_nalu[0] = NAL_SEI;
> +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
> +        sei_nalu[2] = sei->size;
> +        sei_nalu += 3;
> +        memcpy(sei_nalu, sei->data, sei->size);
> +        sei_nalu += sei->size;
> +        sei_nalu[0] = 1; // RBSP
> +    }
> +

SEI data should come after the parameter sets and before the other NAL units.

>     if (is_key_frame) {
>         pkt->flags |= AV_PKT_FLAG_KEY;
>     }
> @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>     CMTime time;
>     CFDictionaryRef frame_dict;
>     CVPixelBufferRef cv_img = NULL;
> +    ExtraSEI *sei = NULL;
>     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
> 
>     if (status) return status;
> @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>         return status;
>     }
> 
> +    if (vtctx->a53_cc) {
> +        sei = av_mallocz(sizeof(*sei));
> +        if (!sei) {
> +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
> +        } else {
> +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data, &sei->size);
> +            if (ret < 0) {
> +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
> +                av_free(sei);
> +                sei = NULL;
> +            }
> +        }
> +    }
> +
>     time = CMTimeMake(frame->pts * avctx->time_base.num, avctx->time_base.den);
>     status = VTCompressionSessionEncodeFrame(
>         vtctx->session,
> @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>         time,
>         kCMTimeInvalid,
>         frame_dict,
> -        NULL,
> +        sei,
>         NULL
>     );
> 
> @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>     bool get_frame;
>     int status;
>     CMSampleBufferRef buf = NULL;
> +    ExtraSEI *sei = NULL;
> 
>     if (frame) {
>         status = vtenc_send_frame(avctx, vtctx, frame);
> @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>         goto end_nopkt;
>     }
> 
> -    status = vtenc_q_pop(vtctx, !frame, &buf);
> +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>     if (status) goto end_nopkt;
>     if (!buf)   goto end_nopkt;
> 
> -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
> +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
> +    if (sei) {
> +        if (sei->data) av_free(sei->data);
> +        av_free(sei);
> +    }
>     CFRelease(buf);
>     if (status) goto end_nopkt;
> 
> @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext   *avctx,
>     if (status)
>         goto pe_cleanup;
> 
> -    status = vtenc_q_pop(vtctx, 0, &buf);
> +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>     if (status) {
>         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>         goto pe_cleanup;
> @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>     { "frames_after", "Other frames will come after the frames in this session. This helps smooth concatenation issues.",
>         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> 
> +    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
> +
>     { NULL },
> };
> 
> -- 
> 2.8.1

Patches should be made against the latest master branch. It doesn’t compile as-is.

Thanks for your work. I look forward to an updated patch.

> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Aman Gupta Sept. 11, 2016, 2:33 a.m. UTC | #6
On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com> wrote:

>
> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net <javascript:;>>
> wrote:
> >
> > From: Aman Gupta <aman@tmm1.net <javascript:;>>
> >
> > ---
> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++
> ++++++++------
> > 1 file changed, 67 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> > index 4345ca3..859dde9 100644
> > --- a/libavcodec/videotoolboxenc.c
> > +++ b/libavcodec/videotoolboxenc.c
> > @@ -32,6 +32,7 @@
> > #include "libavutil/pixdesc.h"
> > #include "internal.h"
> > #include <pthread.h>
> > +#include "h264.h"
> >
> > #if !CONFIG_VT_BT2020
> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
> >
> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
> >
> > +typedef struct ExtraSEI {
> > +  void *data;
> > +  size_t size;
> > +} ExtraSEI;
> > +
> > typedef struct BufNode {
> >     CMSampleBufferRef cm_buffer;
> > +    ExtraSEI *sei;
> >     struct BufNode* next;
> >     int error;
> > } BufNode;
> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
> >     bool flushing;
> >     bool has_b_frames;
> >     bool warned_color_range;
> > +    bool a53_cc;
> > } VTEncContext;
> >
> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx, int
> err)
> >     pthread_mutex_unlock(&vtctx->lock);
> > }
> >
> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
> CMSampleBufferRef *buf)
> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
> CMSampleBufferRef *buf, ExtraSEI **sei)
> > {
> >     BufNode *info;
> >
> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
> wait, CMSampleBufferRef *buf)
> >     pthread_mutex_unlock(&vtctx->lock);
> >
> >     *buf = info->cm_buffer;
> > +    if (sei && *buf) {
> > +        *sei = info->sei;
> > +    } else if (info->sei) {
> > +        if (info->sei->data) av_free(info->sei->data);
> > +        av_free(info->sei);
> > +    }
> >     av_free(info);
> >
> >     vtctx->frame_ct_out++;
> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
> wait, CMSampleBufferRef *buf)
> >     return 0;
> > }
> >
> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer,
> ExtraSEI *sei)
> > {
> >     BufNode *info = av_malloc(sizeof(BufNode));
> >     if (!info) {
> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
> CMSampleBufferRef buffer)
> >
> >     CFRetain(buffer);
> >     info->cm_buffer = buffer;
> > +    info->sei = sei;
> >     info->next = NULL;
> >
> >     pthread_mutex_lock(&vtctx->lock);
> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
> > {
> >     AVCodecContext *avctx = ctx;
> >     VTEncContext   *vtctx = avctx->priv_data;
> > +    ExtraSEI *sei = sourceFrameCtx;
> >
> >     if (vtctx->async_error) {
> >         if(sample_buffer) CFRelease(sample_buffer);
> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
> >         }
> >     }
> >
> > -    vtenc_q_push(vtctx, sample_buffer);
> > +    vtenc_q_push(vtctx, sample_buffer, sei);
> > }
> >
> > static int get_length_code_size(
> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
> > static int vtenc_cm_to_avpacket(
> >     AVCodecContext    *avctx,
> >     CMSampleBufferRef sample_buffer,
> > -    AVPacket          *pkt)
> > +    AVPacket          *pkt,
> > +    ExtraSEI          *sei)
> > {
> >     VTEncContext *vtctx = avctx->priv_data;
> >
> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
> >     size_t  header_size = 0;
> >     size_t  in_buf_size;
> >     size_t  out_buf_size;
> > +    size_t  sei_nalu_size = 0;
> >     int64_t dts_delta;
> >     int64_t time_base_num;
> >     int nalu_count;
> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
> >     if(status)
> >         return status;
> >
> > +    if (sei) {
> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
> > +    }
> > +
> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
> >     out_buf_size = header_size +
> >                    in_buf_size +
> > +                   sei_nalu_size +
> >                    nalu_count * ((int)sizeof(start_code) -
> (int)length_code_size);
> >
> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
> >         length_code_size,
> >         sample_buffer,
> >         pkt->data + header_size,
> > -        pkt->size - header_size
> > +        pkt->size - header_size - sei_nalu_size
> >     );
> >
> >     if (status) {
> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
> >         return status;
> >     }
> >
> > +    if (sei_nalu_size > 0) {
> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
> > +        sei_nalu += sizeof(start_code);
> > +        sei_nalu[0] = NAL_SEI;
> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
> > +        sei_nalu[2] = sei->size;
> > +        sei_nalu += 3;
> > +        memcpy(sei_nalu, sei->data, sei->size);
> > +        sei_nalu += sei->size;
> > +        sei_nalu[0] = 1; // RBSP
> > +    }
> > +
>
> SEI data should come after the parameter sets and before the other NAL
> units.


Thanks. I have no prior experience with the h264 bitstream format so the
help is much appreciated. Any advice on how to get the SEI inserted at the
right spot?

I also am unsure if I need to worry about start code emulation prevention
within the SEI data.


>
> >     if (is_key_frame) {
> >         pkt->flags |= AV_PKT_FLAG_KEY;
> >     }
> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
> >     CMTime time;
> >     CFDictionaryRef frame_dict;
> >     CVPixelBufferRef cv_img = NULL;
> > +    ExtraSEI *sei = NULL;
> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
> >
> >     if (status) return status;
> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext
> *avctx,
> >         return status;
> >     }
> >
> > +    if (vtctx->a53_cc) {
> > +        sei = av_mallocz(sizeof(*sei));
> > +        if (!sei) {
> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed
> captions, skipping\n");
> > +        } else {
> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data,
> &sei->size);
> > +            if (ret < 0) {
> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for
> closed captions, skipping\n");
> > +                av_free(sei);
> > +                sei = NULL;
> > +            }
> > +        }
> > +    }
> > +
> >     time = CMTimeMake(frame->pts * avctx->time_base.num,
> avctx->time_base.den);
> >     status = VTCompressionSessionEncodeFrame(
> >         vtctx->session,
> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
> >         time,
> >         kCMTimeInvalid,
> >         frame_dict,
> > -        NULL,
> > +        sei,
> >         NULL
> >     );
> >
> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
> >     bool get_frame;
> >     int status;
> >     CMSampleBufferRef buf = NULL;
> > +    ExtraSEI *sei = NULL;
> >
> >     if (frame) {
> >         status = vtenc_send_frame(avctx, vtctx, frame);
> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
> >         goto end_nopkt;
> >     }
> >
> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
> >     if (status) goto end_nopkt;
> >     if (!buf)   goto end_nopkt;
> >
> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
> > +    if (sei) {
> > +        if (sei->data) av_free(sei->data);
> > +        av_free(sei);
> > +    }
> >     CFRelease(buf);
> >     if (status) goto end_nopkt;
> >
> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext
>  *avctx,
> >     if (status)
> >         goto pe_cleanup;
> >
> > -    status = vtenc_q_pop(vtctx, 0, &buf);
> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
> >     if (status) {
> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
> >         goto pe_cleanup;
> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
> >     { "frames_after", "Other frames will come after the frames in this
> session. This helps smooth concatenation issues.",
> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> >
> > +    { "a53cc", "Use A53 Closed Captions (if available)",
> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
> > +
> >     { NULL },
> > };
> >
> > --
> > 2.8.1
>
> Patches should be made against the latest master branch. It doesn’t
> compile as-is.


Oops, rebased locally for next patch.


>
> Thanks for your work. I look forward to an updated patch.
>
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org <javascript:;>
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Rick Kern Sept. 19, 2016, 1:27 p.m. UTC | #7
> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> 
> 
> 
> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
> 
> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
> >
> > From: Aman Gupta <aman@tmm1.net <>>
> >
> > ---
> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 67 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> > index 4345ca3..859dde9 100644
> > --- a/libavcodec/videotoolboxenc.c
> > +++ b/libavcodec/videotoolboxenc.c
> > @@ -32,6 +32,7 @@
> > #include "libavutil/pixdesc.h"
> > #include "internal.h"
> > #include <pthread.h>
> > +#include "h264.h"
> >
> > #if !CONFIG_VT_BT2020
> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
> >
> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
> >
> > +typedef struct ExtraSEI {
> > +  void *data;
> > +  size_t size;
> > +} ExtraSEI;
> > +
> > typedef struct BufNode {
> >     CMSampleBufferRef cm_buffer;
> > +    ExtraSEI *sei;
> >     struct BufNode* next;
> >     int error;
> > } BufNode;
> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
> >     bool flushing;
> >     bool has_b_frames;
> >     bool warned_color_range;
> > +    bool a53_cc;
> > } VTEncContext;
> >
> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx, int err)
> >     pthread_mutex_unlock(&vtctx->lock);
> > }
> >
> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, ExtraSEI **sei)
> > {
> >     BufNode *info;
> >
> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
> >     pthread_mutex_unlock(&vtctx->lock);
> >
> >     *buf = info->cm_buffer;
> > +    if (sei && *buf) {
> > +        *sei = info->sei;
> > +    } else if (info->sei) {
> > +        if (info->sei->data) av_free(info->sei->data);
> > +        av_free(info->sei);
> > +    }
> >     av_free(info);
> >
> >     vtctx->frame_ct_out++;
> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
> >     return 0;
> > }
> >
> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer, ExtraSEI *sei)
> > {
> >     BufNode *info = av_malloc(sizeof(BufNode));
> >     if (!info) {
> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
> >
> >     CFRetain(buffer);
> >     info->cm_buffer = buffer;
> > +    info->sei = sei;
> >     info->next = NULL;
> >
> >     pthread_mutex_lock(&vtctx->lock);
> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
> > {
> >     AVCodecContext *avctx = ctx;
> >     VTEncContext   *vtctx = avctx->priv_data;
> > +    ExtraSEI *sei = sourceFrameCtx;
> >
> >     if (vtctx->async_error) {
> >         if(sample_buffer) CFRelease(sample_buffer);
> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
> >         }
> >     }
> >
> > -    vtenc_q_push(vtctx, sample_buffer);
> > +    vtenc_q_push(vtctx, sample_buffer, sei);
> > }
> >
> > static int get_length_code_size(
> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
> > static int vtenc_cm_to_avpacket(
> >     AVCodecContext    *avctx,
> >     CMSampleBufferRef sample_buffer,
> > -    AVPacket          *pkt)
> > +    AVPacket          *pkt,
> > +    ExtraSEI          *sei)
> > {
> >     VTEncContext *vtctx = avctx->priv_data;
> >
> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
> >     size_t  header_size = 0;
> >     size_t  in_buf_size;
> >     size_t  out_buf_size;
> > +    size_t  sei_nalu_size = 0;
> >     int64_t dts_delta;
> >     int64_t time_base_num;
> >     int nalu_count;
> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
> >     if(status)
> >         return status;
> >
> > +    if (sei) {
> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
> > +    }
> > +
> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
> >     out_buf_size = header_size +
> >                    in_buf_size +
> > +                   sei_nalu_size +
> >                    nalu_count * ((int)sizeof(start_code) - (int)length_code_size);
> >
> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
> >         length_code_size,
> >         sample_buffer,
> >         pkt->data + header_size,
> > -        pkt->size - header_size
> > +        pkt->size - header_size - sei_nalu_size
> >     );
> >
> >     if (status) {
> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
> >         return status;
> >     }
> >
> > +    if (sei_nalu_size > 0) {
> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
> > +        sei_nalu += sizeof(start_code);
> > +        sei_nalu[0] = NAL_SEI;
> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
> > +        sei_nalu[2] = sei->size;
> > +        sei_nalu += 3;
> > +        memcpy(sei_nalu, sei->data, sei->size);
> > +        sei_nalu += sei->size;
> > +        sei_nalu[0] = 1; // RBSP
> > +    }
> > +
> 
> SEI data should come after the parameter sets and before the other NAL units.
> 
> Thanks. I have no prior experience with the h264 bitstream format so the help is much appreciated. Any advice on how to get the SEI inserted at the right spot?
> 
> I also am unsure if I need to worry about start code emulation prevention within the SEI data.
The SEI data comes after the parameter sets and AUD NAL units (when they’re present). If I understand correctly, only one SEI NAL unit can be present per access unit (but it can contain multiple SEI messages). When the SEI NAL contains a buffering period SEI message it comes first in the list. The H.264 spec is available from the ITU website and contains the syntax for NAL units, SEI data, etc.

Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d prefer, I can reorder the SEI data later this week.

>  
> 
> >     if (is_key_frame) {
> >         pkt->flags |= AV_PKT_FLAG_KEY;
> >     }
> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
> >     CMTime time;
> >     CFDictionaryRef frame_dict;
> >     CVPixelBufferRef cv_img = NULL;
> > +    ExtraSEI *sei = NULL;
> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
> >
> >     if (status) return status;
> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext *avctx,
> >         return status;
> >     }
> >
> > +    if (vtctx->a53_cc) {
> > +        sei = av_mallocz(sizeof(*sei));
> > +        if (!sei) {
> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
> > +        } else {
> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data, &sei->size);
> > +            if (ret < 0) {
> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
> > +                av_free(sei);
> > +                sei = NULL;
> > +            }
> > +        }
> > +    }
> > +
> >     time = CMTimeMake(frame->pts * avctx->time_base.num, avctx->time_base.den);
> >     status = VTCompressionSessionEncodeFrame(
> >         vtctx->session,
> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
> >         time,
> >         kCMTimeInvalid,
> >         frame_dict,
> > -        NULL,
> > +        sei,
> >         NULL
> >     );
> >
> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
> >     bool get_frame;
> >     int status;
> >     CMSampleBufferRef buf = NULL;
> > +    ExtraSEI *sei = NULL;
> >
> >     if (frame) {
> >         status = vtenc_send_frame(avctx, vtctx, frame);
> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
> >         goto end_nopkt;
> >     }
> >
> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
> >     if (status) goto end_nopkt;
> >     if (!buf)   goto end_nopkt;
> >
> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
> > +    if (sei) {
> > +        if (sei->data) av_free(sei->data);
> > +        av_free(sei);
> > +    }
> >     CFRelease(buf);
> >     if (status) goto end_nopkt;
> >
> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext   *avctx,
> >     if (status)
> >         goto pe_cleanup;
> >
> > -    status = vtenc_q_pop(vtctx, 0, &buf);
> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
> >     if (status) {
> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
> >         goto pe_cleanup;
> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
> >     { "frames_after", "Other frames will come after the frames in this session. This helps smooth concatenation issues.",
> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> >
> > +    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
> > +
> >     { NULL },
> > };
> >
> > --
> > 2.8.1
> 
> Patches should be made against the latest master branch. It doesn’t compile as-is.
> 
> Oops, rebased locally for next patch.
>  
> 
> Thanks for your work. I look forward to an updated patch.
> 
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> >  <>ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
Aman Gupta Sept. 19, 2016, 2:30 p.m. UTC | #8
On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com> wrote:

>
> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net
> <javascript:_e(%7B%7D,'cvml','ffmpeg@tmm1.net');>> wrote:
>
>
>
> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com
> <javascript:_e(%7B%7D,'cvml','kernrj@gmail.com');>> wrote:
>
>>
>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>> >
>> > From: Aman Gupta <aman@tmm1.net>
>> >
>> > ---
>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++
>> ++++++++------
>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> > index 4345ca3..859dde9 100644
>> > --- a/libavcodec/videotoolboxenc.c
>> > +++ b/libavcodec/videotoolboxenc.c
>> > @@ -32,6 +32,7 @@
>> > #include "libavutil/pixdesc.h"
>> > #include "internal.h"
>> > #include <pthread.h>
>> > +#include "h264.h"
>> >
>> > #if !CONFIG_VT_BT2020
>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>> >
>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>> >
>> > +typedef struct ExtraSEI {
>> > +  void *data;
>> > +  size_t size;
>> > +} ExtraSEI;
>> > +
>> > typedef struct BufNode {
>> >     CMSampleBufferRef cm_buffer;
>> > +    ExtraSEI *sei;
>> >     struct BufNode* next;
>> >     int error;
>> > } BufNode;
>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>> >     bool flushing;
>> >     bool has_b_frames;
>> >     bool warned_color_range;
>> > +    bool a53_cc;
>> > } VTEncContext;
>> >
>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx,
>> int err)
>> >     pthread_mutex_unlock(&vtctx->lock);
>> > }
>> >
>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>> CMSampleBufferRef *buf)
>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>> CMSampleBufferRef *buf, ExtraSEI **sei)
>> > {
>> >     BufNode *info;
>> >
>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>> wait, CMSampleBufferRef *buf)
>> >     pthread_mutex_unlock(&vtctx->lock);
>> >
>> >     *buf = info->cm_buffer;
>> > +    if (sei && *buf) {
>> > +        *sei = info->sei;
>> > +    } else if (info->sei) {
>> > +        if (info->sei->data) av_free(info->sei->data);
>> > +        av_free(info->sei);
>> > +    }
>> >     av_free(info);
>> >
>> >     vtctx->frame_ct_out++;
>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>> wait, CMSampleBufferRef *buf)
>> >     return 0;
>> > }
>> >
>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>> buffer, ExtraSEI *sei)
>> > {
>> >     BufNode *info = av_malloc(sizeof(BufNode));
>> >     if (!info) {
>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
>> CMSampleBufferRef buffer)
>> >
>> >     CFRetain(buffer);
>> >     info->cm_buffer = buffer;
>> > +    info->sei = sei;
>> >     info->next = NULL;
>> >
>> >     pthread_mutex_lock(&vtctx->lock);
>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>> > {
>> >     AVCodecContext *avctx = ctx;
>> >     VTEncContext   *vtctx = avctx->priv_data;
>> > +    ExtraSEI *sei = sourceFrameCtx;
>> >
>> >     if (vtctx->async_error) {
>> >         if(sample_buffer) CFRelease(sample_buffer);
>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>> >         }
>> >     }
>> >
>> > -    vtenc_q_push(vtctx, sample_buffer);
>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>> > }
>> >
>> > static int get_length_code_size(
>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>> > static int vtenc_cm_to_avpacket(
>> >     AVCodecContext    *avctx,
>> >     CMSampleBufferRef sample_buffer,
>> > -    AVPacket          *pkt)
>> > +    AVPacket          *pkt,
>> > +    ExtraSEI          *sei)
>> > {
>> >     VTEncContext *vtctx = avctx->priv_data;
>> >
>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>> >     size_t  header_size = 0;
>> >     size_t  in_buf_size;
>> >     size_t  out_buf_size;
>> > +    size_t  sei_nalu_size = 0;
>> >     int64_t dts_delta;
>> >     int64_t time_base_num;
>> >     int nalu_count;
>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>> >     if(status)
>> >         return status;
>> >
>> > +    if (sei) {
>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>> > +    }
>> > +
>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>> >     out_buf_size = header_size +
>> >                    in_buf_size +
>> > +                   sei_nalu_size +
>> >                    nalu_count * ((int)sizeof(start_code) -
>> (int)length_code_size);
>> >
>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>> >         length_code_size,
>> >         sample_buffer,
>> >         pkt->data + header_size,
>> > -        pkt->size - header_size
>> > +        pkt->size - header_size - sei_nalu_size
>> >     );
>> >
>> >     if (status) {
>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>> >         return status;
>> >     }
>> >
>> > +    if (sei_nalu_size > 0) {
>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>> > +        sei_nalu += sizeof(start_code);
>> > +        sei_nalu[0] = NAL_SEI;
>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>> > +        sei_nalu[2] = sei->size;
>> > +        sei_nalu += 3;
>> > +        memcpy(sei_nalu, sei->data, sei->size);
>> > +        sei_nalu += sei->size;
>> > +        sei_nalu[0] = 1; // RBSP
>> > +    }
>> > +
>>
>> SEI data should come after the parameter sets and before the other NAL
>> units.
>
>
> Thanks. I have no prior experience with the h264 bitstream format so the
> help is much appreciated. Any advice on how to get the SEI inserted at the
> right spot?
>
> I also am unsure if I need to worry about start code emulation prevention
> within the SEI data.
>
> The SEI data comes after the parameter sets and AUD NAL units (when
> they’re present). If I understand correctly, only one SEI NAL unit can be
> present per access unit (but it can contain multiple SEI messages). When
> the SEI NAL contains a buffering period SEI message it comes first in the
> list. The H.264 spec is available from the ITU website and contains the
> syntax for NAL units, SEI data, etc.
>
> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d
> prefer, I can reorder the SEI data later this week.
>

No problem. If you get a chance to work up a patch, I would very much
appreciate it.



>
>
>
>>
>> >     if (is_key_frame) {
>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>> >     }
>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext
>> *avctx,
>> >     CMTime time;
>> >     CFDictionaryRef frame_dict;
>> >     CVPixelBufferRef cv_img = NULL;
>> > +    ExtraSEI *sei = NULL;
>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>> >
>> >     if (status) return status;
>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext
>> *avctx,
>> >         return status;
>> >     }
>> >
>> > +    if (vtctx->a53_cc) {
>> > +        sei = av_mallocz(sizeof(*sei));
>> > +        if (!sei) {
>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed
>> captions, skipping\n");
>> > +        } else {
>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data,
>> &sei->size);
>> > +            if (ret < 0) {
>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>> closed captions, skipping\n");
>> > +                av_free(sei);
>> > +                sei = NULL;
>> > +            }
>> > +        }
>> > +    }
>> > +
>> >     time = CMTimeMake(frame->pts * avctx->time_base.num,
>> avctx->time_base.den);
>> >     status = VTCompressionSessionEncodeFrame(
>> >         vtctx->session,
>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext
>> *avctx,
>> >         time,
>> >         kCMTimeInvalid,
>> >         frame_dict,
>> > -        NULL,
>> > +        sei,
>> >         NULL
>> >     );
>> >
>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>> >     bool get_frame;
>> >     int status;
>> >     CMSampleBufferRef buf = NULL;
>> > +    ExtraSEI *sei = NULL;
>> >
>> >     if (frame) {
>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>> >         goto end_nopkt;
>> >     }
>> >
>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>> >     if (status) goto end_nopkt;
>> >     if (!buf)   goto end_nopkt;
>> >
>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>> > +    if (sei) {
>> > +        if (sei->data) av_free(sei->data);
>> > +        av_free(sei);
>> > +    }
>> >     CFRelease(buf);
>> >     if (status) goto end_nopkt;
>> >
>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext
>>  *avctx,
>> >     if (status)
>> >         goto pe_cleanup;
>> >
>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>> >     if (status) {
>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>> >         goto pe_cleanup;
>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>> >     { "frames_after", "Other frames will come after the frames in this
>> session. This helps smooth concatenation issues.",
>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE
>> },
>> >
>> > +    { "a53cc", "Use A53 Closed Captions (if available)",
>> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>> > +
>> >     { NULL },
>> > };
>> >
>> > --
>> > 2.8.1
>>
>> Patches should be made against the latest master branch. It doesn’t
>> compile as-is.
>
>
> Oops, rebased locally for next patch.
>
>
>>
>> Thanks for your work. I look forward to an updated patch.
>>
>> >
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> <javascript:_e(%7B%7D,'cvml','ffmpeg-devel@ffmpeg.org');>
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
>
Rick Kern Oct. 17, 2016, 1:35 p.m. UTC | #9
> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> 
> 
> 
> On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
> 
>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
>> 
>> 
>> 
>> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com <>> wrote:
>> 
>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
>> >
>> > From: Aman Gupta <aman@tmm1.net <>>
>> >
>> > ---
>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++++++++++------
>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> > index 4345ca3..859dde9 100644
>> > --- a/libavcodec/videotoolboxenc.c
>> > +++ b/libavcodec/videotoolboxenc.c
>> > @@ -32,6 +32,7 @@
>> > #include "libavutil/pixdesc.h"
>> > #include "internal.h"
>> > #include <pthread.h>
>> > +#include "h264.h"
>> >
>> > #if !CONFIG_VT_BT2020
>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>> >
>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>> >
>> > +typedef struct ExtraSEI {
>> > +  void *data;
>> > +  size_t size;
>> > +} ExtraSEI;
>> > +
>> > typedef struct BufNode {
>> >     CMSampleBufferRef cm_buffer;
>> > +    ExtraSEI *sei;
>> >     struct BufNode* next;
>> >     int error;
>> > } BufNode;
>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>> >     bool flushing;
>> >     bool has_b_frames;
>> >     bool warned_color_range;
>> > +    bool a53_cc;
>> > } VTEncContext;
>> >
>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx, int err)
>> >     pthread_mutex_unlock(&vtctx->lock);
>> > }
>> >
>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, ExtraSEI **sei)
>> > {
>> >     BufNode *info;
>> >
>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>> >     pthread_mutex_unlock(&vtctx->lock);
>> >
>> >     *buf = info->cm_buffer;
>> > +    if (sei && *buf) {
>> > +        *sei = info->sei;
>> > +    } else if (info->sei) {
>> > +        if (info->sei->data) av_free(info->sei->data);
>> > +        av_free(info->sei);
>> > +    }
>> >     av_free(info);
>> >
>> >     vtctx->frame_ct_out++;
>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>> >     return 0;
>> > }
>> >
>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer, ExtraSEI *sei)
>> > {
>> >     BufNode *info = av_malloc(sizeof(BufNode));
>> >     if (!info) {
>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>> >
>> >     CFRetain(buffer);
>> >     info->cm_buffer = buffer;
>> > +    info->sei = sei;
>> >     info->next = NULL;
>> >
>> >     pthread_mutex_lock(&vtctx->lock);
>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>> > {
>> >     AVCodecContext *avctx = ctx;
>> >     VTEncContext   *vtctx = avctx->priv_data;
>> > +    ExtraSEI *sei = sourceFrameCtx;
>> >
>> >     if (vtctx->async_error) {
>> >         if(sample_buffer) CFRelease(sample_buffer);
>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>> >         }
>> >     }
>> >
>> > -    vtenc_q_push(vtctx, sample_buffer);
>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>> > }
>> >
>> > static int get_length_code_size(
>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>> > static int vtenc_cm_to_avpacket(
>> >     AVCodecContext    *avctx,
>> >     CMSampleBufferRef sample_buffer,
>> > -    AVPacket          *pkt)
>> > +    AVPacket          *pkt,
>> > +    ExtraSEI          *sei)
>> > {
>> >     VTEncContext *vtctx = avctx->priv_data;
>> >
>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>> >     size_t  header_size = 0;
>> >     size_t  in_buf_size;
>> >     size_t  out_buf_size;
>> > +    size_t  sei_nalu_size = 0;
>> >     int64_t dts_delta;
>> >     int64_t time_base_num;
>> >     int nalu_count;
>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>> >     if(status)
>> >         return status;
>> >
>> > +    if (sei) {
>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>> > +    }
>> > +
>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>> >     out_buf_size = header_size +
>> >                    in_buf_size +
>> > +                   sei_nalu_size +
>> >                    nalu_count * ((int)sizeof(start_code) - (int)length_code_size);
>> >
>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>> >         length_code_size,
>> >         sample_buffer,
>> >         pkt->data + header_size,
>> > -        pkt->size - header_size
>> > +        pkt->size - header_size - sei_nalu_size
>> >     );
>> >
>> >     if (status) {
>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>> >         return status;
>> >     }
>> >
>> > +    if (sei_nalu_size > 0) {
>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>> > +        sei_nalu += sizeof(start_code);
>> > +        sei_nalu[0] = NAL_SEI;
>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>> > +        sei_nalu[2] = sei->size;
>> > +        sei_nalu += 3;
>> > +        memcpy(sei_nalu, sei->data, sei->size);
>> > +        sei_nalu += sei->size;
>> > +        sei_nalu[0] = 1; // RBSP
>> > +    }
>> > +
>> 
>> SEI data should come after the parameter sets and before the other NAL units.
>> 
>> Thanks. I have no prior experience with the h264 bitstream format so the help is much appreciated. Any advice on how to get the SEI inserted at the right spot?
>> 
>> I also am unsure if I need to worry about start code emulation prevention within the SEI data.
> The SEI data comes after the parameter sets and AUD NAL units (when they’re present). If I understand correctly, only one SEI NAL unit can be present per access unit (but it can contain multiple SEI messages). When the SEI NAL contains a buffering period SEI message it comes first in the list. The H.264 spec is available from the ITU website and contains the syntax for NAL units, SEI data, etc.
> 
> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d prefer, I can reorder the SEI data later this week.
> 
> No problem. If you get a chance to work up a patch, I would very much appreciate it.

Pushed, thanks.

> 
>  
> 
>>  
>> 
>> >     if (is_key_frame) {
>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>> >     }
>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>> >     CMTime time;
>> >     CFDictionaryRef frame_dict;
>> >     CVPixelBufferRef cv_img = NULL;
>> > +    ExtraSEI *sei = NULL;
>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>> >
>> >     if (status) return status;
>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>> >         return status;
>> >     }
>> >
>> > +    if (vtctx->a53_cc) {
>> > +        sei = av_mallocz(sizeof(*sei));
>> > +        if (!sei) {
>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
>> > +        } else {
>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data, &sei->size);
>> > +            if (ret < 0) {
>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
>> > +                av_free(sei);
>> > +                sei = NULL;
>> > +            }
>> > +        }
>> > +    }
>> > +
>> >     time = CMTimeMake(frame->pts * avctx->time_base.num, avctx->time_base.den);
>> >     status = VTCompressionSessionEncodeFrame(
>> >         vtctx->session,
>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>> >         time,
>> >         kCMTimeInvalid,
>> >         frame_dict,
>> > -        NULL,
>> > +        sei,
>> >         NULL
>> >     );
>> >
>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>> >     bool get_frame;
>> >     int status;
>> >     CMSampleBufferRef buf = NULL;
>> > +    ExtraSEI *sei = NULL;
>> >
>> >     if (frame) {
>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>> >         goto end_nopkt;
>> >     }
>> >
>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>> >     if (status) goto end_nopkt;
>> >     if (!buf)   goto end_nopkt;
>> >
>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>> > +    if (sei) {
>> > +        if (sei->data) av_free(sei->data);
>> > +        av_free(sei);
>> > +    }
>> >     CFRelease(buf);
>> >     if (status) goto end_nopkt;
>> >
>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext   *avctx,
>> >     if (status)
>> >         goto pe_cleanup;
>> >
>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>> >     if (status) {
>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>> >         goto pe_cleanup;
>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>> >     { "frames_after", "Other frames will come after the frames in this session. This helps smooth concatenation issues.",
>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
>> >
>> > +    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>> > +
>> >     { NULL },
>> > };
>> >
>> > --
>> > 2.8.1
>> 
>> Patches should be made against the latest master branch. It doesn’t compile as-is.
>> 
>> Oops, rebased locally for next patch.
>>  
>> 
>> Thanks for your work. I look forward to an updated patch.
>> 
>> >
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> >  <>ffmpeg-devel@ffmpeg.org <>
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
Aman Gupta Oct. 18, 2016, 12:47 a.m. UTC | #10
On Mon, Oct 17, 2016 at 6:35 AM, Richard Kern <kernrj@gmail.com> wrote:

>
> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>
>
>
> On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com> wrote:
>
>>
>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>
>>
>>
>> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com> wrote:
>>
>>>
>>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>> >
>>> > From: Aman Gupta <aman@tmm1.net>
>>> >
>>> > ---
>>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++
>>> ++++++++------
>>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>>> >
>>> > diff --git a/libavcodec/videotoolboxenc.c
>>> b/libavcodec/videotoolboxenc.c
>>> > index 4345ca3..859dde9 100644
>>> > --- a/libavcodec/videotoolboxenc.c
>>> > +++ b/libavcodec/videotoolboxenc.c
>>> > @@ -32,6 +32,7 @@
>>> > #include "libavutil/pixdesc.h"
>>> > #include "internal.h"
>>> > #include <pthread.h>
>>> > +#include "h264.h"
>>> >
>>> > #if !CONFIG_VT_BT2020
>>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>>> >
>>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>>> >
>>> > +typedef struct ExtraSEI {
>>> > +  void *data;
>>> > +  size_t size;
>>> > +} ExtraSEI;
>>> > +
>>> > typedef struct BufNode {
>>> >     CMSampleBufferRef cm_buffer;
>>> > +    ExtraSEI *sei;
>>> >     struct BufNode* next;
>>> >     int error;
>>> > } BufNode;
>>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>>> >     bool flushing;
>>> >     bool has_b_frames;
>>> >     bool warned_color_range;
>>> > +    bool a53_cc;
>>> > } VTEncContext;
>>> >
>>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx,
>>> int err)
>>> >     pthread_mutex_unlock(&vtctx->lock);
>>> > }
>>> >
>>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>>> CMSampleBufferRef *buf)
>>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>>> CMSampleBufferRef *buf, ExtraSEI **sei)
>>> > {
>>> >     BufNode *info;
>>> >
>>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>>> wait, CMSampleBufferRef *buf)
>>> >     pthread_mutex_unlock(&vtctx->lock);
>>> >
>>> >     *buf = info->cm_buffer;
>>> > +    if (sei && *buf) {
>>> > +        *sei = info->sei;
>>> > +    } else if (info->sei) {
>>> > +        if (info->sei->data) av_free(info->sei->data);
>>> > +        av_free(info->sei);
>>> > +    }
>>> >     av_free(info);
>>> >
>>> >     vtctx->frame_ct_out++;
>>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>>> wait, CMSampleBufferRef *buf)
>>> >     return 0;
>>> > }
>>> >
>>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>>> buffer)
>>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>>> buffer, ExtraSEI *sei)
>>> > {
>>> >     BufNode *info = av_malloc(sizeof(BufNode));
>>> >     if (!info) {
>>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
>>> CMSampleBufferRef buffer)
>>> >
>>> >     CFRetain(buffer);
>>> >     info->cm_buffer = buffer;
>>> > +    info->sei = sei;
>>> >     info->next = NULL;
>>> >
>>> >     pthread_mutex_lock(&vtctx->lock);
>>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>>> > {
>>> >     AVCodecContext *avctx = ctx;
>>> >     VTEncContext   *vtctx = avctx->priv_data;
>>> > +    ExtraSEI *sei = sourceFrameCtx;
>>> >
>>> >     if (vtctx->async_error) {
>>> >         if(sample_buffer) CFRelease(sample_buffer);
>>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>>> >         }
>>> >     }
>>> >
>>> > -    vtenc_q_push(vtctx, sample_buffer);
>>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>>> > }
>>> >
>>> > static int get_length_code_size(
>>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>>> > static int vtenc_cm_to_avpacket(
>>> >     AVCodecContext    *avctx,
>>> >     CMSampleBufferRef sample_buffer,
>>> > -    AVPacket          *pkt)
>>> > +    AVPacket          *pkt,
>>> > +    ExtraSEI          *sei)
>>> > {
>>> >     VTEncContext *vtctx = avctx->priv_data;
>>> >
>>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>>> >     size_t  header_size = 0;
>>> >     size_t  in_buf_size;
>>> >     size_t  out_buf_size;
>>> > +    size_t  sei_nalu_size = 0;
>>> >     int64_t dts_delta;
>>> >     int64_t time_base_num;
>>> >     int nalu_count;
>>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>>> >     if(status)
>>> >         return status;
>>> >
>>> > +    if (sei) {
>>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>>> > +    }
>>> > +
>>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>>> >     out_buf_size = header_size +
>>> >                    in_buf_size +
>>> > +                   sei_nalu_size +
>>> >                    nalu_count * ((int)sizeof(start_code) -
>>> (int)length_code_size);
>>> >
>>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>>> >         length_code_size,
>>> >         sample_buffer,
>>> >         pkt->data + header_size,
>>> > -        pkt->size - header_size
>>> > +        pkt->size - header_size - sei_nalu_size
>>> >     );
>>> >
>>> >     if (status) {
>>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>>> >         return status;
>>> >     }
>>> >
>>> > +    if (sei_nalu_size > 0) {
>>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>>> > +        sei_nalu += sizeof(start_code);
>>> > +        sei_nalu[0] = NAL_SEI;
>>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>>> > +        sei_nalu[2] = sei->size;
>>> > +        sei_nalu += 3;
>>> > +        memcpy(sei_nalu, sei->data, sei->size);
>>> > +        sei_nalu += sei->size;
>>> > +        sei_nalu[0] = 1; // RBSP
>>> > +    }
>>> > +
>>>
>>> SEI data should come after the parameter sets and before the other NAL
>>> units.
>>
>>
>> Thanks. I have no prior experience with the h264 bitstream format so the
>> help is much appreciated. Any advice on how to get the SEI inserted at the
>> right spot?
>>
>> I also am unsure if I need to worry about start code emulation prevention
>> within the SEI data.
>>
>> The SEI data comes after the parameter sets and AUD NAL units (when
>> they’re present). If I understand correctly, only one SEI NAL unit can be
>> present per access unit (but it can contain multiple SEI messages). When
>> the SEI NAL contains a buffering period SEI message it comes first in the
>> list. The H.264 spec is available from the ITU website and contains the
>> syntax for NAL units, SEI data, etc.
>>
>> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d
>> prefer, I can reorder the SEI data later this week.
>>
>
> No problem. If you get a chance to work up a patch, I would very much
> appreciate it.
>
>
> Pushed, thanks.
>

Thanks!

I tried a few samples which worked, but on others I'm getting "Error
copying packet data: -1397118274" errors (looks like
AVERROR_BUFFER_TOO_SMALL).

Here's a sample that reproduces the error:
https://s3.amazonaws.com/tmm1/ccaptions/nightlynews2.ts


>
>
>
>>
>>
>>
>>>
>>> >     if (is_key_frame) {
>>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>>> >     }
>>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext
>>> *avctx,
>>> >     CMTime time;
>>> >     CFDictionaryRef frame_dict;
>>> >     CVPixelBufferRef cv_img = NULL;
>>> > +    ExtraSEI *sei = NULL;
>>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>>> >
>>> >     if (status) return status;
>>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext
>>> *avctx,
>>> >         return status;
>>> >     }
>>> >
>>> > +    if (vtctx->a53_cc) {
>>> > +        sei = av_mallocz(sizeof(*sei));
>>> > +        if (!sei) {
>>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed
>>> captions, skipping\n");
>>> > +        } else {
>>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data,
>>> &sei->size);
>>> > +            if (ret < 0) {
>>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>>> closed captions, skipping\n");
>>> > +                av_free(sei);
>>> > +                sei = NULL;
>>> > +            }
>>> > +        }
>>> > +    }
>>> > +
>>> >     time = CMTimeMake(frame->pts * avctx->time_base.num,
>>> avctx->time_base.den);
>>> >     status = VTCompressionSessionEncodeFrame(
>>> >         vtctx->session,
>>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext
>>> *avctx,
>>> >         time,
>>> >         kCMTimeInvalid,
>>> >         frame_dict,
>>> > -        NULL,
>>> > +        sei,
>>> >         NULL
>>> >     );
>>> >
>>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>>> >     bool get_frame;
>>> >     int status;
>>> >     CMSampleBufferRef buf = NULL;
>>> > +    ExtraSEI *sei = NULL;
>>> >
>>> >     if (frame) {
>>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>>> >         goto end_nopkt;
>>> >     }
>>> >
>>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>>> >     if (status) goto end_nopkt;
>>> >     if (!buf)   goto end_nopkt;
>>> >
>>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>>> > +    if (sei) {
>>> > +        if (sei->data) av_free(sei->data);
>>> > +        av_free(sei);
>>> > +    }
>>> >     CFRelease(buf);
>>> >     if (status) goto end_nopkt;
>>> >
>>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext
>>>  *avctx,
>>> >     if (status)
>>> >         goto pe_cleanup;
>>> >
>>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>>> >     if (status) {
>>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>>> >         goto pe_cleanup;
>>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>>> >     { "frames_after", "Other frames will come after the frames in this
>>> session. This helps smooth concatenation issues.",
>>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE
>>> },
>>> >
>>> > +    { "a53cc", "Use A53 Closed Captions (if available)",
>>> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>>> > +
>>> >     { NULL },
>>> > };
>>> >
>>> > --
>>> > 2.8.1
>>>
>>> Patches should be made against the latest master branch. It doesn’t
>>> compile as-is.
>>
>>
>> Oops, rebased locally for next patch.
>>
>>
>>>
>>> Thanks for your work. I look forward to an updated patch.
>>>
>>> >
>>> > _______________________________________________
>>> > ffmpeg-devel mailing list
>>> > ffmpeg-devel@ffmpeg.org
>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
>
Rick Kern Oct. 18, 2016, 12:51 a.m. UTC | #11
> On Oct 17, 2016, at 8:47 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> 
> 
> 
> On Mon, Oct 17, 2016 at 6:35 AM, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
> 
>> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg@tmm1.net <mailto:ffmpeg@tmm1.net>> wrote:
>> 
>> 
>> 
>> On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
>> 
>>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
>>> 
>>> 
>>> 
>>> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com <>> wrote:
>>> 
>>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
>>> >
>>> > From: Aman Gupta <aman@tmm1.net <>>
>>> >
>>> > ---
>>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++++++++++------
>>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>>> >
>>> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>>> > index 4345ca3..859dde9 100644
>>> > --- a/libavcodec/videotoolboxenc.c
>>> > +++ b/libavcodec/videotoolboxenc.c
>>> > @@ -32,6 +32,7 @@
>>> > #include "libavutil/pixdesc.h"
>>> > #include "internal.h"
>>> > #include <pthread.h>
>>> > +#include "h264.h"
>>> >
>>> > #if !CONFIG_VT_BT2020
>>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>>> >
>>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>>> >
>>> > +typedef struct ExtraSEI {
>>> > +  void *data;
>>> > +  size_t size;
>>> > +} ExtraSEI;
>>> > +
>>> > typedef struct BufNode {
>>> >     CMSampleBufferRef cm_buffer;
>>> > +    ExtraSEI *sei;
>>> >     struct BufNode* next;
>>> >     int error;
>>> > } BufNode;
>>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>>> >     bool flushing;
>>> >     bool has_b_frames;
>>> >     bool warned_color_range;
>>> > +    bool a53_cc;
>>> > } VTEncContext;
>>> >
>>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx, int err)
>>> >     pthread_mutex_unlock(&vtctx->lock);
>>> > }
>>> >
>>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, ExtraSEI **sei)
>>> > {
>>> >     BufNode *info;
>>> >
>>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>>> >     pthread_mutex_unlock(&vtctx->lock);
>>> >
>>> >     *buf = info->cm_buffer;
>>> > +    if (sei && *buf) {
>>> > +        *sei = info->sei;
>>> > +    } else if (info->sei) {
>>> > +        if (info->sei->data) av_free(info->sei->data);
>>> > +        av_free(info->sei);
>>> > +    }
>>> >     av_free(info);
>>> >
>>> >     vtctx->frame_ct_out++;
>>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>>> >     return 0;
>>> > }
>>> >
>>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer, ExtraSEI *sei)
>>> > {
>>> >     BufNode *info = av_malloc(sizeof(BufNode));
>>> >     if (!info) {
>>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>>> >
>>> >     CFRetain(buffer);
>>> >     info->cm_buffer = buffer;
>>> > +    info->sei = sei;
>>> >     info->next = NULL;
>>> >
>>> >     pthread_mutex_lock(&vtctx->lock);
>>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>>> > {
>>> >     AVCodecContext *avctx = ctx;
>>> >     VTEncContext   *vtctx = avctx->priv_data;
>>> > +    ExtraSEI *sei = sourceFrameCtx;
>>> >
>>> >     if (vtctx->async_error) {
>>> >         if(sample_buffer) CFRelease(sample_buffer);
>>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>>> >         }
>>> >     }
>>> >
>>> > -    vtenc_q_push(vtctx, sample_buffer);
>>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>>> > }
>>> >
>>> > static int get_length_code_size(
>>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>>> > static int vtenc_cm_to_avpacket(
>>> >     AVCodecContext    *avctx,
>>> >     CMSampleBufferRef sample_buffer,
>>> > -    AVPacket          *pkt)
>>> > +    AVPacket          *pkt,
>>> > +    ExtraSEI          *sei)
>>> > {
>>> >     VTEncContext *vtctx = avctx->priv_data;
>>> >
>>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>>> >     size_t  header_size = 0;
>>> >     size_t  in_buf_size;
>>> >     size_t  out_buf_size;
>>> > +    size_t  sei_nalu_size = 0;
>>> >     int64_t dts_delta;
>>> >     int64_t time_base_num;
>>> >     int nalu_count;
>>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>>> >     if(status)
>>> >         return status;
>>> >
>>> > +    if (sei) {
>>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>>> > +    }
>>> > +
>>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>>> >     out_buf_size = header_size +
>>> >                    in_buf_size +
>>> > +                   sei_nalu_size +
>>> >                    nalu_count * ((int)sizeof(start_code) - (int)length_code_size);
>>> >
>>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>>> >         length_code_size,
>>> >         sample_buffer,
>>> >         pkt->data + header_size,
>>> > -        pkt->size - header_size
>>> > +        pkt->size - header_size - sei_nalu_size
>>> >     );
>>> >
>>> >     if (status) {
>>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>>> >         return status;
>>> >     }
>>> >
>>> > +    if (sei_nalu_size > 0) {
>>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>>> > +        sei_nalu += sizeof(start_code);
>>> > +        sei_nalu[0] = NAL_SEI;
>>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>>> > +        sei_nalu[2] = sei->size;
>>> > +        sei_nalu += 3;
>>> > +        memcpy(sei_nalu, sei->data, sei->size);
>>> > +        sei_nalu += sei->size;
>>> > +        sei_nalu[0] = 1; // RBSP
>>> > +    }
>>> > +
>>> 
>>> SEI data should come after the parameter sets and before the other NAL units.
>>> 
>>> Thanks. I have no prior experience with the h264 bitstream format so the help is much appreciated. Any advice on how to get the SEI inserted at the right spot?
>>> 
>>> I also am unsure if I need to worry about start code emulation prevention within the SEI data.
>> The SEI data comes after the parameter sets and AUD NAL units (when they’re present). If I understand correctly, only one SEI NAL unit can be present per access unit (but it can contain multiple SEI messages). When the SEI NAL contains a buffering period SEI message it comes first in the list. The H.264 spec is available from the ITU website and contains the syntax for NAL units, SEI data, etc.
>> 
>> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d prefer, I can reorder the SEI data later this week.
>> 
>> No problem. If you get a chance to work up a patch, I would very much appreciate it.
> 
> Pushed, thanks.
> 
> Thanks!
> 
> I tried a few samples which worked, but on others I'm getting "Error copying packet data: -1397118274" errors (looks like AVERROR_BUFFER_TOO_SMALL).
> 
> Here's a sample that reproduces the error: https://s3.amazonaws.com/tmm1/ccaptions/nightlynews2.ts <https://s3.amazonaws.com/tmm1/ccaptions/nightlynews2.ts>Thanks, looking into it.

> 
> 
>> 
>>  
>> 
>>>  
>>> 
>>> >     if (is_key_frame) {
>>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>>> >     }
>>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>>> >     CMTime time;
>>> >     CFDictionaryRef frame_dict;
>>> >     CVPixelBufferRef cv_img = NULL;
>>> > +    ExtraSEI *sei = NULL;
>>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>>> >
>>> >     if (status) return status;
>>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>>> >         return status;
>>> >     }
>>> >
>>> > +    if (vtctx->a53_cc) {
>>> > +        sei = av_mallocz(sizeof(*sei));
>>> > +        if (!sei) {
>>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
>>> > +        } else {
>>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data, &sei->size);
>>> > +            if (ret < 0) {
>>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
>>> > +                av_free(sei);
>>> > +                sei = NULL;
>>> > +            }
>>> > +        }
>>> > +    }
>>> > +
>>> >     time = CMTimeMake(frame->pts * avctx->time_base.num, avctx->time_base.den);
>>> >     status = VTCompressionSessionEncodeFrame(
>>> >         vtctx->session,
>>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>>> >         time,
>>> >         kCMTimeInvalid,
>>> >         frame_dict,
>>> > -        NULL,
>>> > +        sei,
>>> >         NULL
>>> >     );
>>> >
>>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>>> >     bool get_frame;
>>> >     int status;
>>> >     CMSampleBufferRef buf = NULL;
>>> > +    ExtraSEI *sei = NULL;
>>> >
>>> >     if (frame) {
>>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>>> >         goto end_nopkt;
>>> >     }
>>> >
>>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>>> >     if (status) goto end_nopkt;
>>> >     if (!buf)   goto end_nopkt;
>>> >
>>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>>> > +    if (sei) {
>>> > +        if (sei->data) av_free(sei->data);
>>> > +        av_free(sei);
>>> > +    }
>>> >     CFRelease(buf);
>>> >     if (status) goto end_nopkt;
>>> >
>>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>> >     if (status)
>>> >         goto pe_cleanup;
>>> >
>>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>>> >     if (status) {
>>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>>> >         goto pe_cleanup;
>>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>>> >     { "frames_after", "Other frames will come after the frames in this session. This helps smooth concatenation issues.",
>>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
>>> >
>>> > +    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>>> > +
>>> >     { NULL },
>>> > };
>>> >
>>> > --
>>> > 2.8.1
>>> 
>>> Patches should be made against the latest master branch. It doesn’t compile as-is.
>>> 
>>> Oops, rebased locally for next patch.
>>>  
>>> 
>>> Thanks for your work. I look forward to an updated patch.
>>> 
>>> >
>>> > _______________________________________________
>>> > ffmpeg-devel mailing list
>>> >  <>ffmpeg-devel@ffmpeg.org <>
>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>
Aman Gupta Oct. 18, 2016, 1:03 a.m. UTC | #12
On Mon, Oct 17, 2016 at 5:51 PM, Richard Kern <kernrj@gmail.com> wrote:

>
> On Oct 17, 2016, at 8:47 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>
>
>
> On Mon, Oct 17, 2016 at 6:35 AM, Richard Kern <kernrj@gmail.com> wrote:
>
>>
>> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>
>>
>>
>> On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com> wrote:
>>
>>>
>>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>>
>>>
>>>
>>> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com> wrote:
>>>
>>>>
>>>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>>> >
>>>> > From: Aman Gupta <aman@tmm1.net>
>>>> >
>>>> > ---
>>>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++
>>>> ++++++++------
>>>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>>>> >
>>>> > diff --git a/libavcodec/videotoolboxenc.c
>>>> b/libavcodec/videotoolboxenc.c
>>>> > index 4345ca3..859dde9 100644
>>>> > --- a/libavcodec/videotoolboxenc.c
>>>> > +++ b/libavcodec/videotoolboxenc.c
>>>> > @@ -32,6 +32,7 @@
>>>> > #include "libavutil/pixdesc.h"
>>>> > #include "internal.h"
>>>> > #include <pthread.h>
>>>> > +#include "h264.h"
>>>> >
>>>> > #if !CONFIG_VT_BT2020
>>>> > # define kCVImageBufferColorPrimaries_ITU_R_2020
>>>>  CFSTR("ITU_R_2020")
>>>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>>>> >
>>>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>>>> >
>>>> > +typedef struct ExtraSEI {
>>>> > +  void *data;
>>>> > +  size_t size;
>>>> > +} ExtraSEI;
>>>> > +
>>>> > typedef struct BufNode {
>>>> >     CMSampleBufferRef cm_buffer;
>>>> > +    ExtraSEI *sei;
>>>> >     struct BufNode* next;
>>>> >     int error;
>>>> > } BufNode;
>>>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>>>> >     bool flushing;
>>>> >     bool has_b_frames;
>>>> >     bool warned_color_range;
>>>> > +    bool a53_cc;
>>>> > } VTEncContext;
>>>> >
>>>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx,
>>>> int err)
>>>> >     pthread_mutex_unlock(&vtctx->lock);
>>>> > }
>>>> >
>>>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>>>> CMSampleBufferRef *buf)
>>>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>>>> CMSampleBufferRef *buf, ExtraSEI **sei)
>>>> > {
>>>> >     BufNode *info;
>>>> >
>>>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>>>> wait, CMSampleBufferRef *buf)
>>>> >     pthread_mutex_unlock(&vtctx->lock);
>>>> >
>>>> >     *buf = info->cm_buffer;
>>>> > +    if (sei && *buf) {
>>>> > +        *sei = info->sei;
>>>> > +    } else if (info->sei) {
>>>> > +        if (info->sei->data) av_free(info->sei->data);
>>>> > +        av_free(info->sei);
>>>> > +    }
>>>> >     av_free(info);
>>>> >
>>>> >     vtctx->frame_ct_out++;
>>>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>>>> wait, CMSampleBufferRef *buf)
>>>> >     return 0;
>>>> > }
>>>> >
>>>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>>>> buffer)
>>>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>>>> buffer, ExtraSEI *sei)
>>>> > {
>>>> >     BufNode *info = av_malloc(sizeof(BufNode));
>>>> >     if (!info) {
>>>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
>>>> CMSampleBufferRef buffer)
>>>> >
>>>> >     CFRetain(buffer);
>>>> >     info->cm_buffer = buffer;
>>>> > +    info->sei = sei;
>>>> >     info->next = NULL;
>>>> >
>>>> >     pthread_mutex_lock(&vtctx->lock);
>>>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>>>> > {
>>>> >     AVCodecContext *avctx = ctx;
>>>> >     VTEncContext   *vtctx = avctx->priv_data;
>>>> > +    ExtraSEI *sei = sourceFrameCtx;
>>>> >
>>>> >     if (vtctx->async_error) {
>>>> >         if(sample_buffer) CFRelease(sample_buffer);
>>>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>>>> >         }
>>>> >     }
>>>> >
>>>> > -    vtenc_q_push(vtctx, sample_buffer);
>>>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>>>> > }
>>>> >
>>>> > static int get_length_code_size(
>>>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>>>> > static int vtenc_cm_to_avpacket(
>>>> >     AVCodecContext    *avctx,
>>>> >     CMSampleBufferRef sample_buffer,
>>>> > -    AVPacket          *pkt)
>>>> > +    AVPacket          *pkt,
>>>> > +    ExtraSEI          *sei)
>>>> > {
>>>> >     VTEncContext *vtctx = avctx->priv_data;
>>>> >
>>>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>>>> >     size_t  header_size = 0;
>>>> >     size_t  in_buf_size;
>>>> >     size_t  out_buf_size;
>>>> > +    size_t  sei_nalu_size = 0;
>>>> >     int64_t dts_delta;
>>>> >     int64_t time_base_num;
>>>> >     int nalu_count;
>>>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>>>> >     if(status)
>>>> >         return status;
>>>> >
>>>> > +    if (sei) {
>>>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>>>> > +    }
>>>> > +
>>>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>>>> >     out_buf_size = header_size +
>>>> >                    in_buf_size +
>>>> > +                   sei_nalu_size +
>>>> >                    nalu_count * ((int)sizeof(start_code) -
>>>> (int)length_code_size);
>>>> >
>>>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>>>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>>>> >         length_code_size,
>>>> >         sample_buffer,
>>>> >         pkt->data + header_size,
>>>> > -        pkt->size - header_size
>>>> > +        pkt->size - header_size - sei_nalu_size
>>>> >     );
>>>> >
>>>> >     if (status) {
>>>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>>>> >         return status;
>>>> >     }
>>>> >
>>>> > +    if (sei_nalu_size > 0) {
>>>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>>>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>>>> > +        sei_nalu += sizeof(start_code);
>>>> > +        sei_nalu[0] = NAL_SEI;
>>>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>>>> > +        sei_nalu[2] = sei->size;
>>>> > +        sei_nalu += 3;
>>>> > +        memcpy(sei_nalu, sei->data, sei->size);
>>>> > +        sei_nalu += sei->size;
>>>> > +        sei_nalu[0] = 1; // RBSP
>>>> > +    }
>>>> > +
>>>>
>>>> SEI data should come after the parameter sets and before the other NAL
>>>> units.
>>>
>>>
>>> Thanks. I have no prior experience with the h264 bitstream format so the
>>> help is much appreciated. Any advice on how to get the SEI inserted at the
>>> right spot?
>>>
>>> I also am unsure if I need to worry about start code emulation
>>> prevention within the SEI data.
>>>
>>> The SEI data comes after the parameter sets and AUD NAL units (when
>>> they’re present). If I understand correctly, only one SEI NAL unit can be
>>> present per access unit (but it can contain multiple SEI messages). When
>>> the SEI NAL contains a buffering period SEI message it comes first in the
>>> list. The H.264 spec is available from the ITU website and contains the
>>> syntax for NAL units, SEI data, etc.
>>>
>>> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d
>>> prefer, I can reorder the SEI data later this week.
>>>
>>
>> No problem. If you get a chance to work up a patch, I would very much
>> appreciate it.
>>
>>
>> Pushed, thanks.
>>
>
> Thanks!
>
> I tried a few samples which worked, but on others I'm getting "Error
> copying packet data: -1397118274" errors (looks like
> AVERROR_BUFFER_TOO_SMALL).
>
> Here's a sample that reproduces the error: https://s3.amazonaws.
> com/tmm1/ccaptions/nightlynews2.ts
>
> Thanks, looking into it.
>

Actually, it looks like it's only happening when I use a specific video
filter that I'm writing. Other filters seem fine, so the bug must be in my
code somewhere.

Aman


>
>
>
>>
>>
>>
>>>
>>>
>>>
>>>>
>>>> >     if (is_key_frame) {
>>>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>>>> >     }
>>>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext
>>>> *avctx,
>>>> >     CMTime time;
>>>> >     CFDictionaryRef frame_dict;
>>>> >     CVPixelBufferRef cv_img = NULL;
>>>> > +    ExtraSEI *sei = NULL;
>>>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>>>> >
>>>> >     if (status) return status;
>>>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext
>>>> *avctx,
>>>> >         return status;
>>>> >     }
>>>> >
>>>> > +    if (vtctx->a53_cc) {
>>>> > +        sei = av_mallocz(sizeof(*sei));
>>>> > +        if (!sei) {
>>>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>>>> closed captions, skipping\n");
>>>> > +        } else {
>>>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data,
>>>> &sei->size);
>>>> > +            if (ret < 0) {
>>>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>>>> closed captions, skipping\n");
>>>> > +                av_free(sei);
>>>> > +                sei = NULL;
>>>> > +            }
>>>> > +        }
>>>> > +    }
>>>> > +
>>>> >     time = CMTimeMake(frame->pts * avctx->time_base.num,
>>>> avctx->time_base.den);
>>>> >     status = VTCompressionSessionEncodeFrame(
>>>> >         vtctx->session,
>>>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext
>>>> *avctx,
>>>> >         time,
>>>> >         kCMTimeInvalid,
>>>> >         frame_dict,
>>>> > -        NULL,
>>>> > +        sei,
>>>> >         NULL
>>>> >     );
>>>> >
>>>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>>>> >     bool get_frame;
>>>> >     int status;
>>>> >     CMSampleBufferRef buf = NULL;
>>>> > +    ExtraSEI *sei = NULL;
>>>> >
>>>> >     if (frame) {
>>>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>>>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>>>> >         goto end_nopkt;
>>>> >     }
>>>> >
>>>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>>>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>>>> >     if (status) goto end_nopkt;
>>>> >     if (!buf)   goto end_nopkt;
>>>> >
>>>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>>>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>>>> > +    if (sei) {
>>>> > +        if (sei->data) av_free(sei->data);
>>>> > +        av_free(sei);
>>>> > +    }
>>>> >     CFRelease(buf);
>>>> >     if (status) goto end_nopkt;
>>>> >
>>>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext
>>>>  *avctx,
>>>> >     if (status)
>>>> >         goto pe_cleanup;
>>>> >
>>>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>>>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>>>> >     if (status) {
>>>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>>>> >         goto pe_cleanup;
>>>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>>>> >     { "frames_after", "Other frames will come after the frames in
>>>> this session. This helps smooth concatenation issues.",
>>>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
>>>> VE },
>>>> >
>>>> > +    { "a53cc", "Use A53 Closed Captions (if available)",
>>>> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>>>> > +
>>>> >     { NULL },
>>>> > };
>>>> >
>>>> > --
>>>> > 2.8.1
>>>>
>>>> Patches should be made against the latest master branch. It doesn’t
>>>> compile as-is.
>>>
>>>
>>> Oops, rebased locally for next patch.
>>>
>>>
>>>>
>>>> Thanks for your work. I look forward to an updated patch.
>>>>
>>>> >
>>>> > _______________________________________________
>>>> > ffmpeg-devel mailing list
>>>> > ffmpeg-devel@ffmpeg.org
>>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>>
>>
>
>
Aman Gupta Oct. 18, 2016, 6:30 p.m. UTC | #13
On Mon, Oct 17, 2016 at 6:03 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:

>
>
> On Mon, Oct 17, 2016 at 5:51 PM, Richard Kern <kernrj@gmail.com> wrote:
>
>>
>> On Oct 17, 2016, at 8:47 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>
>>
>>
>> On Mon, Oct 17, 2016 at 6:35 AM, Richard Kern <kernrj@gmail.com> wrote:
>>
>>>
>>> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>>
>>>
>>>
>>> On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com> wrote:
>>>
>>>>
>>>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>>>
>>>>
>>>>
>>>> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com> wrote:
>>>>
>>>>>
>>>>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>>>>> >
>>>>> > From: Aman Gupta <aman@tmm1.net>
>>>>> >
>>>>> > ---
>>>>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++
>>>>> ++++++++------
>>>>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>>>>> >
>>>>> > diff --git a/libavcodec/videotoolboxenc.c
>>>>> b/libavcodec/videotoolboxenc.c
>>>>> > index 4345ca3..859dde9 100644
>>>>> > --- a/libavcodec/videotoolboxenc.c
>>>>> > +++ b/libavcodec/videotoolboxenc.c
>>>>> > @@ -32,6 +32,7 @@
>>>>> > #include "libavutil/pixdesc.h"
>>>>> > #include "internal.h"
>>>>> > #include <pthread.h>
>>>>> > +#include "h264.h"
>>>>> >
>>>>> > #if !CONFIG_VT_BT2020
>>>>> > # define kCVImageBufferColorPrimaries_ITU_R_2020
>>>>>  CFSTR("ITU_R_2020")
>>>>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>>>>> >
>>>>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>>>>> >
>>>>> > +typedef struct ExtraSEI {
>>>>> > +  void *data;
>>>>> > +  size_t size;
>>>>> > +} ExtraSEI;
>>>>> > +
>>>>> > typedef struct BufNode {
>>>>> >     CMSampleBufferRef cm_buffer;
>>>>> > +    ExtraSEI *sei;
>>>>> >     struct BufNode* next;
>>>>> >     int error;
>>>>> > } BufNode;
>>>>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>>>>> >     bool flushing;
>>>>> >     bool has_b_frames;
>>>>> >     bool warned_color_range;
>>>>> > +    bool a53_cc;
>>>>> > } VTEncContext;
>>>>> >
>>>>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>>>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx,
>>>>> int err)
>>>>> >     pthread_mutex_unlock(&vtctx->lock);
>>>>> > }
>>>>> >
>>>>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>>>>> CMSampleBufferRef *buf)
>>>>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>>>>> CMSampleBufferRef *buf, ExtraSEI **sei)
>>>>> > {
>>>>> >     BufNode *info;
>>>>> >
>>>>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx,
>>>>> bool wait, CMSampleBufferRef *buf)
>>>>> >     pthread_mutex_unlock(&vtctx->lock);
>>>>> >
>>>>> >     *buf = info->cm_buffer;
>>>>> > +    if (sei && *buf) {
>>>>> > +        *sei = info->sei;
>>>>> > +    } else if (info->sei) {
>>>>> > +        if (info->sei->data) av_free(info->sei->data);
>>>>> > +        av_free(info->sei);
>>>>> > +    }
>>>>> >     av_free(info);
>>>>> >
>>>>> >     vtctx->frame_ct_out++;
>>>>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>>>>> wait, CMSampleBufferRef *buf)
>>>>> >     return 0;
>>>>> > }
>>>>> >
>>>>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>>>>> buffer)
>>>>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>>>>> buffer, ExtraSEI *sei)
>>>>> > {
>>>>> >     BufNode *info = av_malloc(sizeof(BufNode));
>>>>> >     if (!info) {
>>>>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
>>>>> CMSampleBufferRef buffer)
>>>>> >
>>>>> >     CFRetain(buffer);
>>>>> >     info->cm_buffer = buffer;
>>>>> > +    info->sei = sei;
>>>>> >     info->next = NULL;
>>>>> >
>>>>> >     pthread_mutex_lock(&vtctx->lock);
>>>>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>>>>> > {
>>>>> >     AVCodecContext *avctx = ctx;
>>>>> >     VTEncContext   *vtctx = avctx->priv_data;
>>>>> > +    ExtraSEI *sei = sourceFrameCtx;
>>>>> >
>>>>> >     if (vtctx->async_error) {
>>>>> >         if(sample_buffer) CFRelease(sample_buffer);
>>>>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>>>>> >         }
>>>>> >     }
>>>>> >
>>>>> > -    vtenc_q_push(vtctx, sample_buffer);
>>>>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>>>>> > }
>>>>> >
>>>>> > static int get_length_code_size(
>>>>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>>>>> > static int vtenc_cm_to_avpacket(
>>>>> >     AVCodecContext    *avctx,
>>>>> >     CMSampleBufferRef sample_buffer,
>>>>> > -    AVPacket          *pkt)
>>>>> > +    AVPacket          *pkt,
>>>>> > +    ExtraSEI          *sei)
>>>>> > {
>>>>> >     VTEncContext *vtctx = avctx->priv_data;
>>>>> >
>>>>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>>>>> >     size_t  header_size = 0;
>>>>> >     size_t  in_buf_size;
>>>>> >     size_t  out_buf_size;
>>>>> > +    size_t  sei_nalu_size = 0;
>>>>> >     int64_t dts_delta;
>>>>> >     int64_t time_base_num;
>>>>> >     int nalu_count;
>>>>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>>>>> >     if(status)
>>>>> >         return status;
>>>>> >
>>>>> > +    if (sei) {
>>>>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>>>>> > +    }
>>>>> > +
>>>>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>>>>> >     out_buf_size = header_size +
>>>>> >                    in_buf_size +
>>>>> > +                   sei_nalu_size +
>>>>> >                    nalu_count * ((int)sizeof(start_code) -
>>>>> (int)length_code_size);
>>>>> >
>>>>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size,
>>>>> out_buf_size);
>>>>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>>>>> >         length_code_size,
>>>>> >         sample_buffer,
>>>>> >         pkt->data + header_size,
>>>>> > -        pkt->size - header_size
>>>>> > +        pkt->size - header_size - sei_nalu_size
>>>>> >     );
>>>>> >
>>>>> >     if (status) {
>>>>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>>>>> >         return status;
>>>>> >     }
>>>>> >
>>>>> > +    if (sei_nalu_size > 0) {
>>>>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>>>>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>>>>> > +        sei_nalu += sizeof(start_code);
>>>>> > +        sei_nalu[0] = NAL_SEI;
>>>>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>>>>> > +        sei_nalu[2] = sei->size;
>>>>> > +        sei_nalu += 3;
>>>>> > +        memcpy(sei_nalu, sei->data, sei->size);
>>>>> > +        sei_nalu += sei->size;
>>>>> > +        sei_nalu[0] = 1; // RBSP
>>>>> > +    }
>>>>> > +
>>>>>
>>>>> SEI data should come after the parameter sets and before the other NAL
>>>>> units.
>>>>
>>>>
>>>> Thanks. I have no prior experience with the h264 bitstream format so
>>>> the help is much appreciated. Any advice on how to get the SEI inserted at
>>>> the right spot?
>>>>
>>>> I also am unsure if I need to worry about start code emulation
>>>> prevention within the SEI data.
>>>>
>>>> The SEI data comes after the parameter sets and AUD NAL units (when
>>>> they’re present). If I understand correctly, only one SEI NAL unit can be
>>>> present per access unit (but it can contain multiple SEI messages). When
>>>> the SEI NAL contains a buffering period SEI message it comes first in the
>>>> list. The H.264 spec is available from the ITU website and contains the
>>>> syntax for NAL units, SEI data, etc.
>>>>
>>>> Sorry I’ve been slow getting back to you. Hope this is helpful. If
>>>> you’d prefer, I can reorder the SEI data later this week.
>>>>
>>>
>>> No problem. If you get a chance to work up a patch, I would very much
>>> appreciate it.
>>>
>>>
>>> Pushed, thanks.
>>>
>>
>> Thanks!
>>
>> I tried a few samples which worked, but on others I'm getting "Error
>> copying packet data: -1397118274" errors (looks like
>> AVERROR_BUFFER_TOO_SMALL).
>>
>> Here's a sample that reproduces the error: https://s3.amazonaws.co
>> m/tmm1/ccaptions/nightlynews2.ts
>>
>> Thanks, looking into it.
>>
>
> Actually, it looks like it's only happening when I use a specific video
> filter that I'm writing. Other filters seem fine, so the bug must be in my
> code somewhere.
>

Still not sure why my filter was triggering the issue, since it's not doing
anything that interesting..

Nevertheless, here's a sample which reproduces the overflow with just a
simple use of -c:v h264_videotoolbox:

  https://s3.amazonaws.com/tmm1/ccaptions/nightlynews3.ts

It looks like an off-by-one somewhere, because if I increase out_buf_size
by 1 the error goes away.

Aman


>
> Aman
>
>
>>
>>
>>
>>>
>>>
>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> >     if (is_key_frame) {
>>>>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>>>>> >     }
>>>>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext
>>>>> *avctx,
>>>>> >     CMTime time;
>>>>> >     CFDictionaryRef frame_dict;
>>>>> >     CVPixelBufferRef cv_img = NULL;
>>>>> > +    ExtraSEI *sei = NULL;
>>>>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>>>>> >
>>>>> >     if (status) return status;
>>>>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext
>>>>> *avctx,
>>>>> >         return status;
>>>>> >     }
>>>>> >
>>>>> > +    if (vtctx->a53_cc) {
>>>>> > +        sei = av_mallocz(sizeof(*sei));
>>>>> > +        if (!sei) {
>>>>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>>>>> closed captions, skipping\n");
>>>>> > +        } else {
>>>>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data,
>>>>> &sei->size);
>>>>> > +            if (ret < 0) {
>>>>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>>>>> closed captions, skipping\n");
>>>>> > +                av_free(sei);
>>>>> > +                sei = NULL;
>>>>> > +            }
>>>>> > +        }
>>>>> > +    }
>>>>> > +
>>>>> >     time = CMTimeMake(frame->pts * avctx->time_base.num,
>>>>> avctx->time_base.den);
>>>>> >     status = VTCompressionSessionEncodeFrame(
>>>>> >         vtctx->session,
>>>>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext
>>>>> *avctx,
>>>>> >         time,
>>>>> >         kCMTimeInvalid,
>>>>> >         frame_dict,
>>>>> > -        NULL,
>>>>> > +        sei,
>>>>> >         NULL
>>>>> >     );
>>>>> >
>>>>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>>>>> >     bool get_frame;
>>>>> >     int status;
>>>>> >     CMSampleBufferRef buf = NULL;
>>>>> > +    ExtraSEI *sei = NULL;
>>>>> >
>>>>> >     if (frame) {
>>>>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>>>>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>>>>> >         goto end_nopkt;
>>>>> >     }
>>>>> >
>>>>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>>>>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>>>>> >     if (status) goto end_nopkt;
>>>>> >     if (!buf)   goto end_nopkt;
>>>>> >
>>>>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>>>>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>>>>> > +    if (sei) {
>>>>> > +        if (sei->data) av_free(sei->data);
>>>>> > +        av_free(sei);
>>>>> > +    }
>>>>> >     CFRelease(buf);
>>>>> >     if (status) goto end_nopkt;
>>>>> >
>>>>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext
>>>>>  *avctx,
>>>>> >     if (status)
>>>>> >         goto pe_cleanup;
>>>>> >
>>>>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>>>>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>>>>> >     if (status) {
>>>>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>>>>> >         goto pe_cleanup;
>>>>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>>>>> >     { "frames_after", "Other frames will come after the frames in
>>>>> this session. This helps smooth concatenation issues.",
>>>>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
>>>>> VE },
>>>>> >
>>>>> > +    { "a53cc", "Use A53 Closed Captions (if available)",
>>>>> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>>>>> > +
>>>>> >     { NULL },
>>>>> > };
>>>>> >
>>>>> > --
>>>>> > 2.8.1
>>>>>
>>>>> Patches should be made against the latest master branch. It doesn’t
>>>>> compile as-is.
>>>>
>>>>
>>>> Oops, rebased locally for next patch.
>>>>
>>>>
>>>>>
>>>>> Thanks for your work. I look forward to an updated patch.
>>>>>
>>>>> >
>>>>> > _______________________________________________
>>>>> > ffmpeg-devel mailing list
>>>>> > ffmpeg-devel@ffmpeg.org
>>>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>>
>>>
>>
>>
>
Rick Kern Oct. 18, 2016, 6:46 p.m. UTC | #14
> On Oct 18, 2016, at 2:30 PM, Aman Gupta <ffmpeg@tmm1.net> wrote:
> 
> 
> 
> On Mon, Oct 17, 2016 at 6:03 PM, Aman Gupta <ffmpeg@tmm1.net <mailto:ffmpeg@tmm1.net>> wrote:
> 
> 
> On Mon, Oct 17, 2016 at 5:51 PM, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
> 
>> On Oct 17, 2016, at 8:47 PM, Aman Gupta <ffmpeg@tmm1.net <mailto:ffmpeg@tmm1.net>> wrote:
>> 
>> 
>> 
>> On Mon, Oct 17, 2016 at 6:35 AM, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
>> 
>>> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg@tmm1.net <mailto:ffmpeg@tmm1.net>> wrote:
>>> 
>>> 
>>> 
>>> On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com <mailto:kernrj@gmail.com>> wrote:
>>> 
>>>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
>>>> 
>>>> 
>>>> 
>>>> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com <>> wrote:
>>>> 
>>>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net <>> wrote:
>>>> >
>>>> > From: Aman Gupta <aman@tmm1.net <>>
>>>> >
>>>> > ---
>>>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++++++++++------
>>>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>>>> >
>>>> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>>>> > index 4345ca3..859dde9 100644
>>>> > --- a/libavcodec/videotoolboxenc.c
>>>> > +++ b/libavcodec/videotoolboxenc.c
>>>> > @@ -32,6 +32,7 @@
>>>> > #include "libavutil/pixdesc.h"
>>>> > #include "internal.h"
>>>> > #include <pthread.h>
>>>> > +#include "h264.h"
>>>> >
>>>> > #if !CONFIG_VT_BT2020
>>>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>>>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>>>> >
>>>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>>>> >
>>>> > +typedef struct ExtraSEI {
>>>> > +  void *data;
>>>> > +  size_t size;
>>>> > +} ExtraSEI;
>>>> > +
>>>> > typedef struct BufNode {
>>>> >     CMSampleBufferRef cm_buffer;
>>>> > +    ExtraSEI *sei;
>>>> >     struct BufNode* next;
>>>> >     int error;
>>>> > } BufNode;
>>>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>>>> >     bool flushing;
>>>> >     bool has_b_frames;
>>>> >     bool warned_color_range;
>>>> > +    bool a53_cc;
>>>> > } VTEncContext;
>>>> >
>>>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx, int err)
>>>> >     pthread_mutex_unlock(&vtctx->lock);
>>>> > }
>>>> >
>>>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>>>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, ExtraSEI **sei)
>>>> > {
>>>> >     BufNode *info;
>>>> >
>>>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>>>> >     pthread_mutex_unlock(&vtctx->lock);
>>>> >
>>>> >     *buf = info->cm_buffer;
>>>> > +    if (sei && *buf) {
>>>> > +        *sei = info->sei;
>>>> > +    } else if (info->sei) {
>>>> > +        if (info->sei->data) av_free(info->sei->data);
>>>> > +        av_free(info->sei);
>>>> > +    }
>>>> >     av_free(info);
>>>> >
>>>> >     vtctx->frame_ct_out++;
>>>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
>>>> >     return 0;
>>>> > }
>>>> >
>>>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>>>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer, ExtraSEI *sei)
>>>> > {
>>>> >     BufNode *info = av_malloc(sizeof(BufNode));
>>>> >     if (!info) {
>>>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>>>> >
>>>> >     CFRetain(buffer);
>>>> >     info->cm_buffer = buffer;
>>>> > +    info->sei = sei;
>>>> >     info->next = NULL;
>>>> >
>>>> >     pthread_mutex_lock(&vtctx->lock);
>>>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>>>> > {
>>>> >     AVCodecContext *avctx = ctx;
>>>> >     VTEncContext   *vtctx = avctx->priv_data;
>>>> > +    ExtraSEI *sei = sourceFrameCtx;
>>>> >
>>>> >     if (vtctx->async_error) {
>>>> >         if(sample_buffer) CFRelease(sample_buffer);
>>>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>>>> >         }
>>>> >     }
>>>> >
>>>> > -    vtenc_q_push(vtctx, sample_buffer);
>>>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>>>> > }
>>>> >
>>>> > static int get_length_code_size(
>>>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>>>> > static int vtenc_cm_to_avpacket(
>>>> >     AVCodecContext    *avctx,
>>>> >     CMSampleBufferRef sample_buffer,
>>>> > -    AVPacket          *pkt)
>>>> > +    AVPacket          *pkt,
>>>> > +    ExtraSEI          *sei)
>>>> > {
>>>> >     VTEncContext *vtctx = avctx->priv_data;
>>>> >
>>>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>>>> >     size_t  header_size = 0;
>>>> >     size_t  in_buf_size;
>>>> >     size_t  out_buf_size;
>>>> > +    size_t  sei_nalu_size = 0;
>>>> >     int64_t dts_delta;
>>>> >     int64_t time_base_num;
>>>> >     int nalu_count;
>>>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>>>> >     if(status)
>>>> >         return status;
>>>> >
>>>> > +    if (sei) {
>>>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>>>> > +    }
>>>> > +
>>>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>>>> >     out_buf_size = header_size +
>>>> >                    in_buf_size +
>>>> > +                   sei_nalu_size +
>>>> >                    nalu_count * ((int)sizeof(start_code) - (int)length_code_size);
>>>> >
>>>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>>>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>>>> >         length_code_size,
>>>> >         sample_buffer,
>>>> >         pkt->data + header_size,
>>>> > -        pkt->size - header_size
>>>> > +        pkt->size - header_size - sei_nalu_size
>>>> >     );
>>>> >
>>>> >     if (status) {
>>>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>>>> >         return status;
>>>> >     }
>>>> >
>>>> > +    if (sei_nalu_size > 0) {
>>>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>>>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>>>> > +        sei_nalu += sizeof(start_code);
>>>> > +        sei_nalu[0] = NAL_SEI;
>>>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>>>> > +        sei_nalu[2] = sei->size;
>>>> > +        sei_nalu += 3;
>>>> > +        memcpy(sei_nalu, sei->data, sei->size);
>>>> > +        sei_nalu += sei->size;
>>>> > +        sei_nalu[0] = 1; // RBSP
>>>> > +    }
>>>> > +
>>>> 
>>>> SEI data should come after the parameter sets and before the other NAL units.
>>>> 
>>>> Thanks. I have no prior experience with the h264 bitstream format so the help is much appreciated. Any advice on how to get the SEI inserted at the right spot?
>>>> 
>>>> I also am unsure if I need to worry about start code emulation prevention within the SEI data.
>>> The SEI data comes after the parameter sets and AUD NAL units (when they’re present). If I understand correctly, only one SEI NAL unit can be present per access unit (but it can contain multiple SEI messages). When the SEI NAL contains a buffering period SEI message it comes first in the list. The H.264 spec is available from the ITU website and contains the syntax for NAL units, SEI data, etc.
>>> 
>>> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d prefer, I can reorder the SEI data later this week.
>>> 
>>> No problem. If you get a chance to work up a patch, I would very much appreciate it.
>> 
>> Pushed, thanks.
>> 
>> Thanks!
>> 
>> I tried a few samples which worked, but on others I'm getting "Error copying packet data: -1397118274" errors (looks like AVERROR_BUFFER_TOO_SMALL).
>> 
>> Here's a sample that reproduces the error: https://s3.amazonaws.com/tmm1/ccaptions/nightlynews2.ts <https://s3.amazonaws.com/tmm1/ccaptions/nightlynews2.ts>Thanks, looking into it.
> 
> Actually, it looks like it's only happening when I use a specific video filter that I'm writing. Other filters seem fine, so the bug must be in my code somewhere.
> 
> Still not sure why my filter was triggering the issue, since it's not doing anything that interesting..
> 
> Nevertheless, here's a sample which reproduces the overflow with just a simple use of -c:v h264_videotoolbox:
> 
>   https://s3.amazonaws.com/tmm1/ccaptions/nightlynews3.ts <https://s3.amazonaws.com/tmm1/ccaptions/nightlynews3.ts>
I haven’t been able to repro with either sample. Can you check that it happens on master with no local changes? If so, please file a bug and post the full command along with output.

> 
> It looks like an off-by-one somewhere, because if I increase out_buf_size by 1 the error goes away.
> 
> Aman 
>  
> 
> Aman
>  
> 
>> 
>> 
>>> 
>>>  
>>> 
>>>>  
>>>> 
>>>> >     if (is_key_frame) {
>>>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>>>> >     }
>>>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>>>> >     CMTime time;
>>>> >     CFDictionaryRef frame_dict;
>>>> >     CVPixelBufferRef cv_img = NULL;
>>>> > +    ExtraSEI *sei = NULL;
>>>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>>>> >
>>>> >     if (status) return status;
>>>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>>>> >         return status;
>>>> >     }
>>>> >
>>>> > +    if (vtctx->a53_cc) {
>>>> > +        sei = av_mallocz(sizeof(*sei));
>>>> > +        if (!sei) {
>>>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
>>>> > +        } else {
>>>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data, &sei->size);
>>>> > +            if (ret < 0) {
>>>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
>>>> > +                av_free(sei);
>>>> > +                sei = NULL;
>>>> > +            }
>>>> > +        }
>>>> > +    }
>>>> > +
>>>> >     time = CMTimeMake(frame->pts * avctx->time_base.num, avctx->time_base.den);
>>>> >     status = VTCompressionSessionEncodeFrame(
>>>> >         vtctx->session,
>>>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext *avctx,
>>>> >         time,
>>>> >         kCMTimeInvalid,
>>>> >         frame_dict,
>>>> > -        NULL,
>>>> > +        sei,
>>>> >         NULL
>>>> >     );
>>>> >
>>>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>>>> >     bool get_frame;
>>>> >     int status;
>>>> >     CMSampleBufferRef buf = NULL;
>>>> > +    ExtraSEI *sei = NULL;
>>>> >
>>>> >     if (frame) {
>>>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>>>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>>>> >         goto end_nopkt;
>>>> >     }
>>>> >
>>>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>>>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>>>> >     if (status) goto end_nopkt;
>>>> >     if (!buf)   goto end_nopkt;
>>>> >
>>>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>>>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>>>> > +    if (sei) {
>>>> > +        if (sei->data) av_free(sei->data);
>>>> > +        av_free(sei);
>>>> > +    }
>>>> >     CFRelease(buf);
>>>> >     if (status) goto end_nopkt;
>>>> >
>>>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext   *avctx,
>>>> >     if (status)
>>>> >         goto pe_cleanup;
>>>> >
>>>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>>>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>>>> >     if (status) {
>>>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>>>> >         goto pe_cleanup;
>>>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>>>> >     { "frames_after", "Other frames will come after the frames in this session. This helps smooth concatenation issues.",
>>>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
>>>> >
>>>> > +    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>>>> > +
>>>> >     { NULL },
>>>> > };
>>>> >
>>>> > --
>>>> > 2.8.1
>>>> 
>>>> Patches should be made against the latest master branch. It doesn’t compile as-is.
>>>> 
>>>> Oops, rebased locally for next patch.
>>>>  
>>>> 
>>>> Thanks for your work. I look forward to an updated patch.
>>>> 
>>>> >
>>>> > _______________________________________________
>>>> > ffmpeg-devel mailing list
>>>> >  <>ffmpeg-devel@ffmpeg.org <>
>>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
Aman Gupta Oct. 24, 2016, 6:17 a.m. UTC | #15
On Monday, September 19, 2016, Richard Kern <kernrj@gmail.com> wrote:

>
> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg@tmm1.net
> <javascript:_e(%7B%7D,'cvml','ffmpeg@tmm1.net');>> wrote:
>
>
>
> On Sunday, September 11, 2016, Richard Kern <kernrj@gmail.com
> <javascript:_e(%7B%7D,'cvml','kernrj@gmail.com');>> wrote:
>
>>
>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg@tmm1.net> wrote:
>> >
>> > From: Aman Gupta <aman@tmm1.net>
>> >
>> > ---
>> > libavcodec/videotoolboxenc.c | 76 ++++++++++++++++++++++++++++++
>> ++++++++------
>> > 1 file changed, 67 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> > index 4345ca3..859dde9 100644
>> > --- a/libavcodec/videotoolboxenc.c
>> > +++ b/libavcodec/videotoolboxenc.c
>> > @@ -32,6 +32,7 @@
>> > #include "libavutil/pixdesc.h"
>> > #include "internal.h"
>> > #include <pthread.h>
>> > +#include "h264.h"
>> >
>> > #if !CONFIG_VT_BT2020
>> > # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
>> > @@ -55,8 +56,14 @@ typedef enum VTH264Entropy{
>> >
>> > static const uint8_t start_code[] = { 0, 0, 0, 1 };
>> >
>> > +typedef struct ExtraSEI {
>> > +  void *data;
>> > +  size_t size;
>> > +} ExtraSEI;
>> > +
>> > typedef struct BufNode {
>> >     CMSampleBufferRef cm_buffer;
>> > +    ExtraSEI *sei;
>> >     struct BufNode* next;
>> >     int error;
>> > } BufNode;
>> > @@ -94,6 +101,7 @@ typedef struct VTEncContext {
>> >     bool flushing;
>> >     bool has_b_frames;
>> >     bool warned_color_range;
>> > +    bool a53_cc;
>> > } VTEncContext;
>> >
>> > static int vtenc_populate_extradata(AVCodecContext   *avctx,
>> > @@ -136,7 +144,7 @@ static void set_async_error(VTEncContext *vtctx,
>> int err)
>> >     pthread_mutex_unlock(&vtctx->lock);
>> > }
>> >
>> > -static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>> CMSampleBufferRef *buf)
>> > +static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
>> CMSampleBufferRef *buf, ExtraSEI **sei)
>> > {
>> >     BufNode *info;
>> >
>> > @@ -173,6 +181,12 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>> wait, CMSampleBufferRef *buf)
>> >     pthread_mutex_unlock(&vtctx->lock);
>> >
>> >     *buf = info->cm_buffer;
>> > +    if (sei && *buf) {
>> > +        *sei = info->sei;
>> > +    } else if (info->sei) {
>> > +        if (info->sei->data) av_free(info->sei->data);
>> > +        av_free(info->sei);
>> > +    }
>> >     av_free(info);
>> >
>> >     vtctx->frame_ct_out++;
>> > @@ -180,7 +194,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool
>> wait, CMSampleBufferRef *buf)
>> >     return 0;
>> > }
>> >
>> > -static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
>> > +static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef
>> buffer, ExtraSEI *sei)
>> > {
>> >     BufNode *info = av_malloc(sizeof(BufNode));
>> >     if (!info) {
>> > @@ -190,6 +204,7 @@ static void vtenc_q_push(VTEncContext *vtctx,
>> CMSampleBufferRef buffer)
>> >
>> >     CFRetain(buffer);
>> >     info->cm_buffer = buffer;
>> > +    info->sei = sei;
>> >     info->next = NULL;
>> >
>> >     pthread_mutex_lock(&vtctx->lock);
>> > @@ -420,6 +435,7 @@ static void vtenc_output_callback(
>> > {
>> >     AVCodecContext *avctx = ctx;
>> >     VTEncContext   *vtctx = avctx->priv_data;
>> > +    ExtraSEI *sei = sourceFrameCtx;
>> >
>> >     if (vtctx->async_error) {
>> >         if(sample_buffer) CFRelease(sample_buffer);
>> > @@ -440,7 +456,7 @@ static void vtenc_output_callback(
>> >         }
>> >     }
>> >
>> > -    vtenc_q_push(vtctx, sample_buffer);
>> > +    vtenc_q_push(vtctx, sample_buffer, sei);
>> > }
>> >
>> > static int get_length_code_size(
>> > @@ -1258,7 +1274,8 @@ static int copy_replace_length_codes(
>> > static int vtenc_cm_to_avpacket(
>> >     AVCodecContext    *avctx,
>> >     CMSampleBufferRef sample_buffer,
>> > -    AVPacket          *pkt)
>> > +    AVPacket          *pkt,
>> > +    ExtraSEI          *sei)
>> > {
>> >     VTEncContext *vtctx = avctx->priv_data;
>> >
>> > @@ -1269,6 +1286,7 @@ static int vtenc_cm_to_avpacket(
>> >     size_t  header_size = 0;
>> >     size_t  in_buf_size;
>> >     size_t  out_buf_size;
>> > +    size_t  sei_nalu_size = 0;
>> >     int64_t dts_delta;
>> >     int64_t time_base_num;
>> >     int nalu_count;
>> > @@ -1298,9 +1316,14 @@ static int vtenc_cm_to_avpacket(
>> >     if(status)
>> >         return status;
>> >
>> > +    if (sei) {
>> > +        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
>> > +    }
>> > +
>> >     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
>> >     out_buf_size = header_size +
>> >                    in_buf_size +
>> > +                   sei_nalu_size +
>> >                    nalu_count * ((int)sizeof(start_code) -
>> (int)length_code_size);
>> >
>> >     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
>> > @@ -1317,7 +1340,7 @@ static int vtenc_cm_to_avpacket(
>> >         length_code_size,
>> >         sample_buffer,
>> >         pkt->data + header_size,
>> > -        pkt->size - header_size
>> > +        pkt->size - header_size - sei_nalu_size
>> >     );
>> >
>> >     if (status) {
>> > @@ -1325,6 +1348,19 @@ static int vtenc_cm_to_avpacket(
>> >         return status;
>> >     }
>> >
>> > +    if (sei_nalu_size > 0) {
>> > +        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
>> > +        memcpy(sei_nalu, start_code, sizeof(start_code));
>> > +        sei_nalu += sizeof(start_code);
>> > +        sei_nalu[0] = NAL_SEI;
>> > +        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
>> > +        sei_nalu[2] = sei->size;
>> > +        sei_nalu += 3;
>> > +        memcpy(sei_nalu, sei->data, sei->size);
>> > +        sei_nalu += sei->size;
>> > +        sei_nalu[0] = 1; // RBSP
>> > +    }
>> > +
>>
>> SEI data should come after the parameter sets and before the other NAL
>> units.
>
>
> Thanks. I have no prior experience with the h264 bitstream format so the
> help is much appreciated. Any advice on how to get the SEI inserted at the
> right spot?
>
> I also am unsure if I need to worry about start code emulation prevention
> within the SEI data.
>
> The SEI data comes after the parameter sets and AUD NAL units (when
> they’re present). If I understand correctly, only one SEI NAL unit can be
> present per access unit (but it can contain multiple SEI messages). When
> the SEI NAL contains a buffering period SEI message it comes first in the
> list. The H.264 spec is available from the ITU website and contains the
> syntax for NAL units, SEI data, etc.
>
> Sorry I’ve been slow getting back to you. Hope this is helpful. If you’d
> prefer, I can reorder the SEI data later this week.
>

No problem. If you get a chance to work up a patch, I would very much
appreciate it.



>
>
>
>>
>> >     if (is_key_frame) {
>> >         pkt->flags |= AV_PKT_FLAG_KEY;
>> >     }
>> > @@ -1707,6 +1743,7 @@ static int vtenc_send_frame(AVCodecContext
>> *avctx,
>> >     CMTime time;
>> >     CFDictionaryRef frame_dict;
>> >     CVPixelBufferRef cv_img = NULL;
>> > +    ExtraSEI *sei = NULL;
>> >     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
>> >
>> >     if (status) return status;
>> > @@ -1717,6 +1754,20 @@ static int vtenc_send_frame(AVCodecContext
>> *avctx,
>> >         return status;
>> >     }
>> >
>> > +    if (vtctx->a53_cc) {
>> > +        sei = av_mallocz(sizeof(*sei));
>> > +        if (!sei) {
>> > +            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed
>> captions, skipping\n");
>> > +        } else {
>> > +            int ret = ff_alloc_a53_sei(frame, 0, &sei->data,
>> &sei->size);
>> > +            if (ret < 0) {
>> > +                av_log(avctx, AV_LOG_ERROR, "Not enough memory for
>> closed captions, skipping\n");
>> > +                av_free(sei);
>> > +                sei = NULL;
>> > +            }
>> > +        }
>> > +    }
>> > +
>> >     time = CMTimeMake(frame->pts * avctx->time_base.num,
>> avctx->time_base.den);
>> >     status = VTCompressionSessionEncodeFrame(
>> >         vtctx->session,
>> > @@ -1724,7 +1775,7 @@ static int vtenc_send_frame(AVCodecContext
>> *avctx,
>> >         time,
>> >         kCMTimeInvalid,
>> >         frame_dict,
>> > -        NULL,
>> > +        sei,
>> >         NULL
>> >     );
>> >
>> > @@ -1749,6 +1800,7 @@ static av_cold int vtenc_frame(
>> >     bool get_frame;
>> >     int status;
>> >     CMSampleBufferRef buf = NULL;
>> > +    ExtraSEI *sei = NULL;
>> >
>> >     if (frame) {
>> >         status = vtenc_send_frame(avctx, vtctx, frame);
>> > @@ -1785,11 +1837,15 @@ static av_cold int vtenc_frame(
>> >         goto end_nopkt;
>> >     }
>> >
>> > -    status = vtenc_q_pop(vtctx, !frame, &buf);
>> > +    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
>> >     if (status) goto end_nopkt;
>> >     if (!buf)   goto end_nopkt;
>> >
>> > -    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
>> > +    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
>> > +    if (sei) {
>> > +        if (sei->data) av_free(sei->data);
>> > +        av_free(sei);
>> > +    }
>> >     CFRelease(buf);
>> >     if (status) goto end_nopkt;
>> >
>> > @@ -1878,7 +1934,7 @@ static int vtenc_populate_extradata(AVCodecContext
>>  *avctx,
>> >     if (status)
>> >         goto pe_cleanup;
>> >
>> > -    status = vtenc_q_pop(vtctx, 0, &buf);
>> > +    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
>> >     if (status) {
>> >         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
>> >         goto pe_cleanup;
>> > @@ -1976,6 +2032,8 @@ static const AVOption options[] = {
>> >     { "frames_after", "Other frames will come after the frames in this
>> session. This helps smooth concatenation issues.",
>> >         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE
>> },
>> >
>> > +    { "a53cc", "Use A53 Closed Captions (if available)",
>> OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
>> > +
>> >     { NULL },
>> > };
>> >
>> > --
>> > 2.8.1
>>
>> Patches should be made against the latest master branch. It doesn’t
>> compile as-is.
>
>
> Oops, rebased locally for next patch.
>
>
>>
>> Thanks for your work. I look forward to an updated patch.
>>
>> >
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> <javascript:_e(%7B%7D,'cvml','ffmpeg-devel@ffmpeg.org');>
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
>
diff mbox

Patch

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 4345ca3..859dde9 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -32,6 +32,7 @@ 
 #include "libavutil/pixdesc.h"
 #include "internal.h"
 #include <pthread.h>
+#include "h264.h"
 
 #if !CONFIG_VT_BT2020
 # define kCVImageBufferColorPrimaries_ITU_R_2020   CFSTR("ITU_R_2020")
@@ -55,8 +56,14 @@  typedef enum VTH264Entropy{
 
 static const uint8_t start_code[] = { 0, 0, 0, 1 };
 
+typedef struct ExtraSEI {
+  void *data;
+  size_t size;
+} ExtraSEI;
+
 typedef struct BufNode {
     CMSampleBufferRef cm_buffer;
+    ExtraSEI *sei;
     struct BufNode* next;
     int error;
 } BufNode;
@@ -94,6 +101,7 @@  typedef struct VTEncContext {
     bool flushing;
     bool has_b_frames;
     bool warned_color_range;
+    bool a53_cc;
 } VTEncContext;
 
 static int vtenc_populate_extradata(AVCodecContext   *avctx,
@@ -136,7 +144,7 @@  static void set_async_error(VTEncContext *vtctx, int err)
     pthread_mutex_unlock(&vtctx->lock);
 }
 
-static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
+static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, ExtraSEI **sei)
 {
     BufNode *info;
 
@@ -173,6 +181,12 @@  static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
     pthread_mutex_unlock(&vtctx->lock);
 
     *buf = info->cm_buffer;
+    if (sei && *buf) {
+        *sei = info->sei;
+    } else if (info->sei) {
+        if (info->sei->data) av_free(info->sei->data);
+        av_free(info->sei);
+    }
     av_free(info);
 
     vtctx->frame_ct_out++;
@@ -180,7 +194,7 @@  static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf)
     return 0;
 }
 
-static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
+static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer, ExtraSEI *sei)
 {
     BufNode *info = av_malloc(sizeof(BufNode));
     if (!info) {
@@ -190,6 +204,7 @@  static void vtenc_q_push(VTEncContext *vtctx, CMSampleBufferRef buffer)
 
     CFRetain(buffer);
     info->cm_buffer = buffer;
+    info->sei = sei;
     info->next = NULL;
 
     pthread_mutex_lock(&vtctx->lock);
@@ -420,6 +435,7 @@  static void vtenc_output_callback(
 {
     AVCodecContext *avctx = ctx;
     VTEncContext   *vtctx = avctx->priv_data;
+    ExtraSEI *sei = sourceFrameCtx;
 
     if (vtctx->async_error) {
         if(sample_buffer) CFRelease(sample_buffer);
@@ -440,7 +456,7 @@  static void vtenc_output_callback(
         }
     }
 
-    vtenc_q_push(vtctx, sample_buffer);
+    vtenc_q_push(vtctx, sample_buffer, sei);
 }
 
 static int get_length_code_size(
@@ -1258,7 +1274,8 @@  static int copy_replace_length_codes(
 static int vtenc_cm_to_avpacket(
     AVCodecContext    *avctx,
     CMSampleBufferRef sample_buffer,
-    AVPacket          *pkt)
+    AVPacket          *pkt,
+    ExtraSEI          *sei)
 {
     VTEncContext *vtctx = avctx->priv_data;
 
@@ -1269,6 +1286,7 @@  static int vtenc_cm_to_avpacket(
     size_t  header_size = 0;
     size_t  in_buf_size;
     size_t  out_buf_size;
+    size_t  sei_nalu_size = 0;
     int64_t dts_delta;
     int64_t time_base_num;
     int nalu_count;
@@ -1298,9 +1316,14 @@  static int vtenc_cm_to_avpacket(
     if(status)
         return status;
 
+    if (sei) {
+        sei_nalu_size = sizeof(start_code) + 3 + sei->size + 1;
+    }
+
     in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer);
     out_buf_size = header_size +
                    in_buf_size +
+                   sei_nalu_size +
                    nalu_count * ((int)sizeof(start_code) - (int)length_code_size);
 
     status = ff_alloc_packet2(avctx, pkt, out_buf_size, out_buf_size);
@@ -1317,7 +1340,7 @@  static int vtenc_cm_to_avpacket(
         length_code_size,
         sample_buffer,
         pkt->data + header_size,
-        pkt->size - header_size
+        pkt->size - header_size - sei_nalu_size
     );
 
     if (status) {
@@ -1325,6 +1348,19 @@  static int vtenc_cm_to_avpacket(
         return status;
     }
 
+    if (sei_nalu_size > 0) {
+        uint8_t *sei_nalu = pkt->data + pkt->size - sei_nalu_size;
+        memcpy(sei_nalu, start_code, sizeof(start_code));
+        sei_nalu += sizeof(start_code);
+        sei_nalu[0] = NAL_SEI;
+        sei_nalu[1] = SEI_TYPE_USER_DATA_REGISTERED;
+        sei_nalu[2] = sei->size;
+        sei_nalu += 3;
+        memcpy(sei_nalu, sei->data, sei->size);
+        sei_nalu += sei->size;
+        sei_nalu[0] = 1; // RBSP
+    }
+
     if (is_key_frame) {
         pkt->flags |= AV_PKT_FLAG_KEY;
     }
@@ -1707,6 +1743,7 @@  static int vtenc_send_frame(AVCodecContext *avctx,
     CMTime time;
     CFDictionaryRef frame_dict;
     CVPixelBufferRef cv_img = NULL;
+    ExtraSEI *sei = NULL;
     int status = create_cv_pixel_buffer(avctx, frame, &cv_img);
 
     if (status) return status;
@@ -1717,6 +1754,20 @@  static int vtenc_send_frame(AVCodecContext *avctx,
         return status;
     }
 
+    if (vtctx->a53_cc) {
+        sei = av_mallocz(sizeof(*sei));
+        if (!sei) {
+            av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
+        } else {
+            int ret = ff_alloc_a53_sei(frame, 0, &sei->data, &sei->size);
+            if (ret < 0) {
+                av_log(avctx, AV_LOG_ERROR, "Not enough memory for closed captions, skipping\n");
+                av_free(sei);
+                sei = NULL;
+            }
+        }
+    }
+
     time = CMTimeMake(frame->pts * avctx->time_base.num, avctx->time_base.den);
     status = VTCompressionSessionEncodeFrame(
         vtctx->session,
@@ -1724,7 +1775,7 @@  static int vtenc_send_frame(AVCodecContext *avctx,
         time,
         kCMTimeInvalid,
         frame_dict,
-        NULL,
+        sei,
         NULL
     );
 
@@ -1749,6 +1800,7 @@  static av_cold int vtenc_frame(
     bool get_frame;
     int status;
     CMSampleBufferRef buf = NULL;
+    ExtraSEI *sei = NULL;
 
     if (frame) {
         status = vtenc_send_frame(avctx, vtctx, frame);
@@ -1785,11 +1837,15 @@  static av_cold int vtenc_frame(
         goto end_nopkt;
     }
 
-    status = vtenc_q_pop(vtctx, !frame, &buf);
+    status = vtenc_q_pop(vtctx, !frame, &buf, &sei);
     if (status) goto end_nopkt;
     if (!buf)   goto end_nopkt;
 
-    status = vtenc_cm_to_avpacket(avctx, buf, pkt);
+    status = vtenc_cm_to_avpacket(avctx, buf, pkt, sei);
+    if (sei) {
+        if (sei->data) av_free(sei->data);
+        av_free(sei);
+    }
     CFRelease(buf);
     if (status) goto end_nopkt;
 
@@ -1878,7 +1934,7 @@  static int vtenc_populate_extradata(AVCodecContext   *avctx,
     if (status)
         goto pe_cleanup;
 
-    status = vtenc_q_pop(vtctx, 0, &buf);
+    status = vtenc_q_pop(vtctx, 0, &buf, NULL);
     if (status) {
         av_log(avctx, AV_LOG_ERROR, "popping: %d\n", status);
         goto pe_cleanup;
@@ -1976,6 +2032,8 @@  static const AVOption options[] = {
     { "frames_after", "Other frames will come after the frames in this session. This helps smooth concatenation issues.",
         OFFSET(frames_after), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
 
+    { "a53cc", "Use A53 Closed Captions (if available)", OFFSET(a53_cc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VE },
+
     { NULL },
 };