[FFmpeg-devel] [PATCH 2/5] h264_metadata: Add support for A/53 closed captions

[Mark Thompson]

On 26/03/18 01:18, James Almer wrote:
> On 3/25/2018 2:41 PM, Mark Thompson wrote:
>> Allows insertion (from side data), extraction (to side data), and removal
>> of closed captions in SEI messages.
>> ---
>>  libavcodec/Makefile            |   2 +-
>>  libavcodec/h264_metadata_bsf.c | 138 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 139 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index aaef6c3ab8..cfde104055 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -1044,7 +1044,7 @@ OBJS-$(CONFIG_DCA_CORE_BSF)               += dca_core_bsf.o
>>  OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)      += extract_extradata_bsf.o    \
>>                                               h2645_parse.o
>>  OBJS-$(CONFIG_FILTER_UNITS_BSF)           += filter_units_bsf.o
>> -OBJS-$(CONFIG_H264_METADATA_BSF)          += h264_metadata_bsf.o
>> +OBJS-$(CONFIG_H264_METADATA_BSF)          += h264_metadata_bsf.o cbs_misc.o
>>  OBJS-$(CONFIG_H264_MP4TOANNEXB_BSF)       += h264_mp4toannexb_bsf.o
>>  OBJS-$(CONFIG_H264_REDUNDANT_PPS_BSF)     += h264_redundant_pps_bsf.o
>>  OBJS-$(CONFIG_HAPQA_EXTRACT_BSF)          += hapqa_extract_bsf.o hap.o
>> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
>> index 27053dbdcf..d79e6c0c87 100644
>> --- a/libavcodec/h264_metadata_bsf.c
>> +++ b/libavcodec/h264_metadata_bsf.c
>> @@ -24,6 +24,7 @@
>>  #include "bsf.h"
>>  #include "cbs.h"
>>  #include "cbs_h264.h"
>> +#include "cbs_misc.h"
>>  #include "h264.h"
>>  #include "h264_sei.h"
>>  
>> @@ -74,6 +75,8 @@ typedef struct H264MetadataContext {
>>      int display_orientation;
>>      double rotate;
>>      int flip;
>> +
>> +    int a53_cc;
>>  } H264MetadataContext;
>>  
>>  
>> @@ -222,6 +225,8 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out)
>>      int err, i, j, has_sps;
>>      uint8_t *displaymatrix_side_data = NULL;
>>      size_t displaymatrix_side_data_size = 0;
>> +    uint8_t *a53_side_data = NULL;
>> +    size_t a53_side_data_size = 0;
>>  
>>      err = ff_bsf_get_packet(bsf, &in);
>>      if (err < 0)
>> @@ -516,6 +521,116 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out)
>>          }
>>      }
>>  
>> +    if (ctx->a53_cc == INSERT) {
> 
> This function is becoming pretty big. Could you split it, either before
> or after this patch, for readability sake? One function per each
> AVOption with pass/insert/remove, basically.

Not very fun because they all interact to some degree (the easily-splittable SPS update is already a separate function).  I might look at this later.

- Mark