[FFmpeg-devel] [PATCH] lavc/videotoolboxenc: implement a53cc
Aman Gupta
ffmpeg at tmm1.net
Tue Oct 18 21:30:34 EEST 2016
On Mon, Oct 17, 2016 at 6:03 PM, Aman Gupta <ffmpeg at tmm1.net> wrote:
>
>
> On Mon, Oct 17, 2016 at 5:51 PM, Richard Kern <kernrj at gmail.com> wrote:
>
>>
>> On Oct 17, 2016, at 8:47 PM, Aman Gupta <ffmpeg at tmm1.net> wrote:
>>
>>
>>
>> On Mon, Oct 17, 2016 at 6:35 AM, Richard Kern <kernrj at gmail.com> wrote:
>>
>>>
>>> On Sep 19, 2016, at 10:30 AM, Aman Gupta <ffmpeg at tmm1.net> wrote:
>>>
>>>
>>>
>>> On Monday, September 19, 2016, Richard Kern <kernrj at gmail.com> wrote:
>>>
>>>>
>>>> On Sep 10, 2016, at 10:33 PM, Aman Gupta <ffmpeg at tmm1.net> wrote:
>>>>
>>>>
>>>>
>>>> On Sunday, September 11, 2016, Richard Kern <kernrj at gmail.com> wrote:
>>>>
>>>>>
>>>>> > On Sep 8, 2016, at 4:19 AM, Aman Gupta <ffmpeg at tmm1.net> wrote:
>>>>> >
>>>>> > From: Aman Gupta <aman at 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 at ffmpeg.org
>>>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>>
>>>
>>
>>
>
More information about the ffmpeg-devel
mailing list