Message ID | 20180325174137.14749-2-sw@jkqxz.net |
---|---|
State | New |
Headers | show |
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.
Is there any documentation on the side data file format? On Sun, Mar 25, 2018 at 6:18 PM, James Almer <jamrial@gmail.com> 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. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 26/03/18 14:33, Alex Giladi wrote: > Is there any documentation on the side data file format? The format for the side data is the existing one used in AV_PKT_DATA_A53_CC. The documentation for that could probably be improved - basically it's just a sequence of the 3-byte CC data packets as defined by CEA-708. > On Sun, Mar 25, 2018 at 6:18 PM, James Almer <jamrial@gmail.com> 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.
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
On Sun, Mar 25, 2018 at 06:41:34PM +0100, 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(-) This breaks build on mips-linux-gnu-gcc-4.4 (Debian 4.4.5-8) 4.4.5 In file included from src/libavcodec/cbs_misc.c:25: src/libavcodec/cbs_misc.h:73: warning: declaration does not declare anything src/libavcodec/cbs_misc.h:86: warning: declaration does not declare anything In file included from src/libavcodec/cbs_misc.c:57: src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_atsc_user_data’: src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_user_data’: src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ In file included from src/libavcodec/cbs_misc.c:82: src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_atsc_user_data’: src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_user_data’: src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ src/libavcodec/cbs_misc.c: In function ‘ff_cbs_read_a53_cc_side_data’: src/libavcodec/cbs_misc.c:153: error: unknown field ‘atsc’ specified in initializer src/libavcodec/cbs_misc.c:153: error: extra brace group at end of initializer src/libavcodec/cbs_misc.c:153: error: (near initialization for ‘(anonymous)’) src/libavcodec/cbs_misc.c:156: error: extra brace group at end of initializer src/libavcodec/cbs_misc.c:156: error: (near initialization for ‘(anonymous)’) src/libavcodec/cbs_misc.c:165: warning: excess elements in struct initializer src/libavcodec/cbs_misc.c:165: warning: (near initialization for ‘(anonymous)’) src/libavcodec/cbs_misc.c:167: error: ‘A53UserData’ has no member named ‘atsc’ src/libavcodec/cbs_misc.c: In function ‘ff_cbs_write_a53_cc_side_data’: src/libavcodec/cbs_misc.c:193: error: ‘A53UserData’ has no member named ‘atsc’ src/libavcodec/cbs_misc.c:196: error: ‘A53UserData’ has no member named ‘atsc’ CC libavcodec/dpxenc.o CC libavcodec/dpx_parser.o make: *** [libavcodec/cbs_misc.o] Error 1 make: *** Waiting for unfinished jobs.... src/libavcodec/dnxhddec.c:146: warning: ‘dnxhd_decode_init_thread_copy’ defined but not used src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_init’: src/libavcodec/dnxhdenc.c:518: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) src/libavcodec/dnxhdenc.c:519: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) src/libavcodec/dnxhdenc.c: In function ‘dnxhd_load_picture’: src/libavcodec/dnxhdenc.c:1272: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_picture’: src/libavcodec/dnxhdenc.c:1337: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) src/libavcodec/dpxenc.c: In function ‘encode_frame’: src/libavcodec/dpxenc.c:180: warning: ‘need_align’ may be used uninitialized in this function src/libavcodec/dpxenc.c:180: warning: ‘len’ may be used uninitialized in this function [...]
On 27/03/18 01:20, Michael Niedermayer wrote: > On Sun, Mar 25, 2018 at 06:41:34PM +0100, 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(-) > > This breaks build on mips-linux-gnu-gcc-4.4 (Debian 4.4.5-8) 4.4.5 > > > In file included from src/libavcodec/cbs_misc.c:25: > src/libavcodec/cbs_misc.h:73: warning: declaration does not declare anything > src/libavcodec/cbs_misc.h:86: warning: declaration does not declare anything > In file included from src/libavcodec/cbs_misc.c:57: > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_atsc_user_data’: > src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ > src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_user_data’: > src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ > src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ > In file included from src/libavcodec/cbs_misc.c:82: > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_atsc_user_data’: > src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ > src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_user_data’: > src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ > src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ > src/libavcodec/cbs_misc.c: In function ‘ff_cbs_read_a53_cc_side_data’: > src/libavcodec/cbs_misc.c:153: error: unknown field ‘atsc’ specified in initializer > src/libavcodec/cbs_misc.c:153: error: extra brace group at end of initializer > src/libavcodec/cbs_misc.c:153: error: (near initialization for ‘(anonymous)’) > src/libavcodec/cbs_misc.c:156: error: extra brace group at end of initializer > src/libavcodec/cbs_misc.c:156: error: (near initialization for ‘(anonymous)’) > src/libavcodec/cbs_misc.c:165: warning: excess elements in struct initializer > src/libavcodec/cbs_misc.c:165: warning: (near initialization for ‘(anonymous)’) > src/libavcodec/cbs_misc.c:167: error: ‘A53UserData’ has no member named ‘atsc’ > src/libavcodec/cbs_misc.c: In function ‘ff_cbs_write_a53_cc_side_data’: > src/libavcodec/cbs_misc.c:193: error: ‘A53UserData’ has no member named ‘atsc’ > src/libavcodec/cbs_misc.c:196: error: ‘A53UserData’ has no member named ‘atsc’ > CC libavcodec/dpxenc.o > CC libavcodec/dpx_parser.o > make: *** [libavcodec/cbs_misc.o] Error 1 > make: *** Waiting for unfinished jobs.... > src/libavcodec/dnxhddec.c:146: warning: ‘dnxhd_decode_init_thread_copy’ defined but not used > src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_init’: > src/libavcodec/dnxhdenc.c:518: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > src/libavcodec/dnxhdenc.c:519: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > src/libavcodec/dnxhdenc.c: In function ‘dnxhd_load_picture’: > src/libavcodec/dnxhdenc.c:1272: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_picture’: > src/libavcodec/dnxhdenc.c:1337: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > src/libavcodec/dpxenc.c: In function ‘encode_frame’: > src/libavcodec/dpxenc.c:180: warning: ‘need_align’ may be used uninitialized in this function > src/libavcodec/dpxenc.c:180: warning: ‘len’ may be used uninitialized in this function > > > [...] Do you happen to know what set of compilers lack anonymous union support like this? It's quite tempting to add a configure option to just not build it for them. - Mark
On 3/26/2018 9:31 PM, Mark Thompson wrote: > On 27/03/18 01:20, Michael Niedermayer wrote: >> On Sun, Mar 25, 2018 at 06:41:34PM +0100, 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(-) >> >> This breaks build on mips-linux-gnu-gcc-4.4 (Debian 4.4.5-8) 4.4.5 >> >> >> In file included from src/libavcodec/cbs_misc.c:25: >> src/libavcodec/cbs_misc.h:73: warning: declaration does not declare anything >> src/libavcodec/cbs_misc.h:86: warning: declaration does not declare anything >> In file included from src/libavcodec/cbs_misc.c:57: >> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_atsc_user_data’: >> src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ >> src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ >> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_user_data’: >> src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ >> src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ >> In file included from src/libavcodec/cbs_misc.c:82: >> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_atsc_user_data’: >> src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ >> src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ >> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_user_data’: >> src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ >> src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ >> src/libavcodec/cbs_misc.c: In function ‘ff_cbs_read_a53_cc_side_data’: >> src/libavcodec/cbs_misc.c:153: error: unknown field ‘atsc’ specified in initializer >> src/libavcodec/cbs_misc.c:153: error: extra brace group at end of initializer >> src/libavcodec/cbs_misc.c:153: error: (near initialization for ‘(anonymous)’) >> src/libavcodec/cbs_misc.c:156: error: extra brace group at end of initializer >> src/libavcodec/cbs_misc.c:156: error: (near initialization for ‘(anonymous)’) >> src/libavcodec/cbs_misc.c:165: warning: excess elements in struct initializer >> src/libavcodec/cbs_misc.c:165: warning: (near initialization for ‘(anonymous)’) >> src/libavcodec/cbs_misc.c:167: error: ‘A53UserData’ has no member named ‘atsc’ >> src/libavcodec/cbs_misc.c: In function ‘ff_cbs_write_a53_cc_side_data’: >> src/libavcodec/cbs_misc.c:193: error: ‘A53UserData’ has no member named ‘atsc’ >> src/libavcodec/cbs_misc.c:196: error: ‘A53UserData’ has no member named ‘atsc’ >> CC libavcodec/dpxenc.o >> CC libavcodec/dpx_parser.o >> make: *** [libavcodec/cbs_misc.o] Error 1 >> make: *** Waiting for unfinished jobs.... >> src/libavcodec/dnxhddec.c:146: warning: ‘dnxhd_decode_init_thread_copy’ defined but not used >> src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_init’: >> src/libavcodec/dnxhdenc.c:518: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >> src/libavcodec/dnxhdenc.c:519: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >> src/libavcodec/dnxhdenc.c: In function ‘dnxhd_load_picture’: >> src/libavcodec/dnxhdenc.c:1272: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >> src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_picture’: >> src/libavcodec/dnxhdenc.c:1337: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >> src/libavcodec/dpxenc.c: In function ‘encode_frame’: >> src/libavcodec/dpxenc.c:180: warning: ‘need_align’ may be used uninitialized in this function >> src/libavcodec/dpxenc.c:180: warning: ‘len’ may be used uninitialized in this function >> >> >> [...] > > Do you happen to know what set of compilers lack anonymous union support like this? It's quite tempting to add a configure option to just not build it for them. Apparently, GCC prior to 4.6 See commit b93d96a07be40f8e5d267d55fe961285586c0fd7
On Tue, Mar 27, 2018 at 01:31:43AM +0100, Mark Thompson wrote: > On 27/03/18 01:20, Michael Niedermayer wrote: > > On Sun, Mar 25, 2018 at 06:41:34PM +0100, 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(-) > > > > This breaks build on mips-linux-gnu-gcc-4.4 (Debian 4.4.5-8) 4.4.5 > > > > > > In file included from src/libavcodec/cbs_misc.c:25: > > src/libavcodec/cbs_misc.h:73: warning: declaration does not declare anything > > src/libavcodec/cbs_misc.h:86: warning: declaration does not declare anything > > In file included from src/libavcodec/cbs_misc.c:57: > > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_atsc_user_data’: > > src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ > > src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ > > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_user_data’: > > src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ > > src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ > > In file included from src/libavcodec/cbs_misc.c:82: > > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_atsc_user_data’: > > src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ > > src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ > > src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_user_data’: > > src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ > > src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ > > src/libavcodec/cbs_misc.c: In function ‘ff_cbs_read_a53_cc_side_data’: > > src/libavcodec/cbs_misc.c:153: error: unknown field ‘atsc’ specified in initializer > > src/libavcodec/cbs_misc.c:153: error: extra brace group at end of initializer > > src/libavcodec/cbs_misc.c:153: error: (near initialization for ‘(anonymous)’) > > src/libavcodec/cbs_misc.c:156: error: extra brace group at end of initializer > > src/libavcodec/cbs_misc.c:156: error: (near initialization for ‘(anonymous)’) > > src/libavcodec/cbs_misc.c:165: warning: excess elements in struct initializer > > src/libavcodec/cbs_misc.c:165: warning: (near initialization for ‘(anonymous)’) > > src/libavcodec/cbs_misc.c:167: error: ‘A53UserData’ has no member named ‘atsc’ > > src/libavcodec/cbs_misc.c: In function ‘ff_cbs_write_a53_cc_side_data’: > > src/libavcodec/cbs_misc.c:193: error: ‘A53UserData’ has no member named ‘atsc’ > > src/libavcodec/cbs_misc.c:196: error: ‘A53UserData’ has no member named ‘atsc’ > > CC libavcodec/dpxenc.o > > CC libavcodec/dpx_parser.o > > make: *** [libavcodec/cbs_misc.o] Error 1 > > make: *** Waiting for unfinished jobs.... > > src/libavcodec/dnxhddec.c:146: warning: ‘dnxhd_decode_init_thread_copy’ defined but not used > > src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_init’: > > src/libavcodec/dnxhdenc.c:518: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > > src/libavcodec/dnxhdenc.c:519: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > > src/libavcodec/dnxhdenc.c: In function ‘dnxhd_load_picture’: > > src/libavcodec/dnxhdenc.c:1272: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > > src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_picture’: > > src/libavcodec/dnxhdenc.c:1337: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) > > src/libavcodec/dpxenc.c: In function ‘encode_frame’: > > src/libavcodec/dpxenc.c:180: warning: ‘need_align’ may be used uninitialized in this function > > src/libavcodec/dpxenc.c:180: warning: ‘len’ may be used uninitialized in this function > > > > > > [...] > > Do you happen to know what set of compilers lack anonymous union support like this? It's quite tempting to add a configure option to just not build it for them. I dont know which compilers, in the sense of names and versions but anonymous unions are AFAIK C11, so compilers that support just C99 could fail. I think we should not drop support for pure C99 compilers yet. Doing that would also require better understanding the impact ... thanks [...]
On 27/03/18 21:38, Michael Niedermayer wrote: > On Tue, Mar 27, 2018 at 01:31:43AM +0100, Mark Thompson wrote: >> On 27/03/18 01:20, Michael Niedermayer wrote: >>> On Sun, Mar 25, 2018 at 06:41:34PM +0100, 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(-) >>> >>> This breaks build on mips-linux-gnu-gcc-4.4 (Debian 4.4.5-8) 4.4.5 >>> >>> >>> In file included from src/libavcodec/cbs_misc.c:25: >>> src/libavcodec/cbs_misc.h:73: warning: declaration does not declare anything >>> src/libavcodec/cbs_misc.h:86: warning: declaration does not declare anything >>> In file included from src/libavcodec/cbs_misc.c:57: >>> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_atsc_user_data’: >>> src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ >>> src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ >>> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_read_a53_user_data’: >>> src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ >>> src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ >>> In file included from src/libavcodec/cbs_misc.c:82: >>> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_atsc_user_data’: >>> src/libavcodec/cbs_misc_syntax_template.c:102: error: ‘A53ATSCUserData’ has no member named ‘cc_data’ >>> src/libavcodec/cbs_misc_syntax_template.c:104: error: ‘A53ATSCUserData’ has no member named ‘bar_data’ >>> src/libavcodec/cbs_misc_syntax_template.c: In function ‘cbs_misc_write_a53_user_data’: >>> src/libavcodec/cbs_misc_syntax_template.c:140: error: ‘A53UserData’ has no member named ‘atsc’ >>> src/libavcodec/cbs_misc_syntax_template.c:142: error: ‘A53UserData’ has no member named ‘afd’ >>> src/libavcodec/cbs_misc.c: In function ‘ff_cbs_read_a53_cc_side_data’: >>> src/libavcodec/cbs_misc.c:153: error: unknown field ‘atsc’ specified in initializer >>> src/libavcodec/cbs_misc.c:153: error: extra brace group at end of initializer >>> src/libavcodec/cbs_misc.c:153: error: (near initialization for ‘(anonymous)’) >>> src/libavcodec/cbs_misc.c:156: error: extra brace group at end of initializer >>> src/libavcodec/cbs_misc.c:156: error: (near initialization for ‘(anonymous)’) >>> src/libavcodec/cbs_misc.c:165: warning: excess elements in struct initializer >>> src/libavcodec/cbs_misc.c:165: warning: (near initialization for ‘(anonymous)’) >>> src/libavcodec/cbs_misc.c:167: error: ‘A53UserData’ has no member named ‘atsc’ >>> src/libavcodec/cbs_misc.c: In function ‘ff_cbs_write_a53_cc_side_data’: >>> src/libavcodec/cbs_misc.c:193: error: ‘A53UserData’ has no member named ‘atsc’ >>> src/libavcodec/cbs_misc.c:196: error: ‘A53UserData’ has no member named ‘atsc’ >>> CC libavcodec/dpxenc.o >>> CC libavcodec/dpx_parser.o >>> make: *** [libavcodec/cbs_misc.o] Error 1 >>> make: *** Waiting for unfinished jobs.... >>> src/libavcodec/dnxhddec.c:146: warning: ‘dnxhd_decode_init_thread_copy’ defined but not used >>> src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_init’: >>> src/libavcodec/dnxhdenc.c:518: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >>> src/libavcodec/dnxhdenc.c:519: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >>> src/libavcodec/dnxhdenc.c: In function ‘dnxhd_load_picture’: >>> src/libavcodec/dnxhdenc.c:1272: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >>> src/libavcodec/dnxhdenc.c: In function ‘dnxhd_encode_picture’: >>> src/libavcodec/dnxhdenc.c:1337: warning: ‘coded_frame’ is deprecated (declared at src/libavcodec/avcodec.h:2760) >>> src/libavcodec/dpxenc.c: In function ‘encode_frame’: >>> src/libavcodec/dpxenc.c:180: warning: ‘need_align’ may be used uninitialized in this function >>> src/libavcodec/dpxenc.c:180: warning: ‘len’ may be used uninitialized in this function >>> >>> >>> [...] >> >> Do you happen to know what set of compilers lack anonymous union support like this? It's quite tempting to add a configure option to just not build it for them. > > I dont know which compilers, in the sense of names and versions but > > anonymous unions are AFAIK C11, so compilers that support just C99 could > fail. > I think we should not drop support for pure C99 compilers yet. Doing that > would also require better understanding the impact ... To clarify, I wasn't intending to suggest that we kill pre-C11 support entirely (that would be rather extreme, given we don't even require all of C99). Rather, I meant adding a configure test something like: test_cc <<EOF && enable anonymous_union struct a { union { int b; }; }; int f(void) { struct a x = { .b = 1 }; return x.b; } EOF and then new components could depend on anonymous_union in configure to avoid having to add additional nesting depth to structures where it isn't wanted. If it is only a problem with past compilers then I don't think this would be an unreasonable constraint for a new component. On the other hand, if current compilers don't support it then maybe not. Thanks, - Mark
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) { + uint8_t *data; + int size; + + data = av_packet_get_side_data(in, AV_PKT_DATA_A53_CC, &size); + if (data) { + A53UserData a53_ud; + + err = ff_cbs_read_a53_cc_side_data(ctx->cbc, &a53_ud, + data, size); + if (err < 0) { + av_log(bsf, AV_LOG_WARNING, "Invalid A/53 closed captions " + "in packet side data dropped.\n"); + } else { + H264RawSEIPayload payload = { + .payload_type = H264_SEI_TYPE_USER_DATA_REGISTERED, + }; + H264RawSEIUserDataRegistered *udr = + &payload.payload.user_data_registered; + size_t size = 9 + 3 * a53_ud.atsc.cc_data.cc_count; + + udr->data_ref = av_buffer_alloc(2 + size); + if (!udr->data_ref) { + err = AVERROR(ENOMEM); + goto fail; + } + udr->data = udr->data_ref->data; + + udr->itu_t_t35_country_code = 181; + AV_WB16(udr->data, 49); // provider_code + + err = ff_cbs_write_a53_user_data(ctx->cbc, udr->data + 2, + &size, &a53_ud); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to write " + "A/53 user data.\n"); + av_buffer_unref(&udr->data_ref); + goto fail; + } + udr->data_length = size + 2; + + err = ff_cbs_h264_add_sei_message(ctx->cbc, au, &payload); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to add A/53 user data " + "SEI message to access unit.\n"); + av_buffer_unref(&udr->data_ref); + goto fail; + } + } + } + + } else if (ctx->a53_cc == REMOVE || ctx->a53_cc == EXTRACT) { + for (i = 0; i < au->nb_units; i++) { + H264RawSEI *sei; + if (au->units[i].type != H264_NAL_SEI) + continue; + sei = au->units[i].content; + + for (j = 0; j < sei->payload_count; j++) { + H264RawSEIUserDataRegistered *udr; + A53UserData a53_ud; + + if (sei->payload[j].payload_type != + H264_SEI_TYPE_USER_DATA_REGISTERED) + continue; + udr = &sei->payload[j].payload.user_data_registered; + if (udr->data_length < 6) { + // Can't be relevant. + continue; + } + + err = ff_cbs_read_a53_user_data(ctx->cbc, &a53_ud, + udr->data + 2, + udr->data_length - 2); + if (err < 0) { + // Invalid or something completely different. + continue; + } + if (a53_ud.user_identifier != A53_USER_IDENTIFIER_ATSC || + a53_ud.atsc.user_data_type_code != + A53_USER_DATA_TYPE_CODE_CC_DATA) { + // Valid but something else (e.g. AFD). + continue; + } + + if (ctx->a53_cc == REMOVE) { + err = ff_cbs_h264_delete_sei_message(ctx->cbc, au, + &au->units[i], j); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to delete " + "A/53 CC SEI message.\n"); + goto fail; + } + --i; + break; + } else if(ctx->a53_cc == EXTRACT) { + err = ff_cbs_write_a53_cc_side_data(ctx->cbc, + &a53_side_data, + &a53_side_data_size, + &a53_ud); + if (err < 0) { + av_log(bsf, AV_LOG_ERROR, "Failed to write " + "A/53 user data for packet side data.\n"); + goto fail; + } + } + } + } + } + err = ff_cbs_write_packet(ctx->cbc, out, au); if (err < 0) { av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n"); @@ -537,6 +652,16 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out) } displaymatrix_side_data = NULL; } + if (a53_side_data) { + err = av_packet_add_side_data(out, AV_PKT_DATA_A53_CC, + a53_side_data, a53_side_data_size); + if (err) { + av_log(bsf, AV_LOG_ERROR, "Failed to attach extracted A/53 " + "side data to packet.\n"); + goto fail; + } + a53_side_data = NULL; + } ctx->done_first_au = 1; @@ -544,6 +669,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *out) fail: ff_cbs_fragment_uninit(ctx->cbc, au); av_freep(&displaymatrix_side_data); + av_freep(&a53_side_data); if (err < 0) av_packet_unref(out); @@ -684,6 +810,18 @@ static const AVOption h264_metadata_options[] = { 0, AV_OPT_TYPE_CONST, { .i64 = FLIP_VERTICAL }, .flags = FLAGS, .unit = "flip" }, + { "a53_cc", "A/53 Closed Captions in SEI NAL units", + OFFSET(a53_cc), AV_OPT_TYPE_INT, + { .i64 = PASS }, PASS, EXTRACT, FLAGS, "a53_cc" }, + { "pass", NULL, 0, AV_OPT_TYPE_CONST, + { .i64 = PASS }, .flags = FLAGS, .unit = "a53_cc" }, + { "insert", NULL, 0, AV_OPT_TYPE_CONST, + { .i64 = INSERT }, .flags = FLAGS, .unit = "a53_cc" }, + { "remove", NULL, 0, AV_OPT_TYPE_CONST, + { .i64 = REMOVE }, .flags = FLAGS, .unit = "a53_cc" }, + { "extract", NULL, 0, AV_OPT_TYPE_CONST, + { .i64 = EXTRACT }, .flags = FLAGS, .unit = "a53_cc" }, + { NULL } };