> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Mark > Thompson > Sent: Sunday, March 11, 2018 8:38 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter > > On 08/03/18 04:01, James Almer wrote: > > On 3/6/2018 3:49 PM, Mark Thompson wrote: > >> This can remove units with types in or not in a given set from a stream. > >> For example, it can be used to remove all non-VCL NAL units from an > >> H.264 or > >> H.265 stream. > >> --- > >> On 06/03/18 17:27, Hendrik Leppkes wrote: > >>> On Tue, Mar 6, 2018 at 3:51 PM, Eran Kornblau <eran.kornb...@kaltura.com> > >>> wrote: > >>>> Hi all, > >>>> > >>>> The attached patch adds a parameter that enables the user to choose > >>>> which AVC/HEVC NAL units to include in the output. > >>>> The parameter is supplied as a bitmask in order to keep things simple. > >>>> > >>>> A short background on why we need it - in our transcoding process, > >>>> we partition the video in chunks, the chunks are transcoded in > >>>> parallel and packaged in MPEG-TS container. The transcoded TS > >>>> chunks are then concatenated and packaged in MP4. These MP4 files are > >>>> later repackaged on-the-fly to various protocols (HLS/DASH etc.) using > >>>> our JIT packager. > >>>> For performance reasons (can get into more detail if anyone's > >>>> interested...), when packaging the MP4 to DASH/CENC, we configure the > >>>> packager to assume that each AVC frame contains exactly one NAL unit. > >>>> The problem is that the transition through MPEG-TS adds additional > >>>> NAL units (NAL AUD before each frame + SPS/PPS before each key frame), > >>>> and this assumption fails. > >>>> Using the attached patch we can pass '-nal_types_mask 0x3e' which will > >>>> make ffmpeg output only VCL NALs in the stream. > >>>> > >>> > >>> Having such logic in one single muxer is not something we really > >>> like around here. Next time someone needs something similar for > >>> another codec, you're stuck re-implementing it. > >>> > >>> To achieve the same effect, Mark Thompson quickly wipped up a > >>> Bitstream Filter using his CBS framework which achieves the same > >>> result. He'll be sending that patch to the mailing list in a while. > >> The suggested use-case would be '-bsf:v filter_units=pass_types=1-5' for > >> H.264 or '-bsf:v filter_units=pass_types=0-31' for H.265. > >> > >> (Also note that filters already exist for some individual parts of > >> this: h264_metadata can remove AUDs, extract_extradata can remove > >> parameter sets.) > >> > >> - Mark > >> > >> > >> libavcodec/Makefile | 1 + > >> libavcodec/bitstream_filters.c | 1 + > >> libavcodec/filter_units_bsf.c | 250 > >> +++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 252 insertions(+) > >> create mode 100644 libavcodec/filter_units_bsf.c > >> > > > > Can you write some minimal documentation with the two above examples? > > Done. > > >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile index > >> b496f0d..b99bdce 100644 > >> --- a/libavcodec/Makefile > >> +++ b/libavcodec/Makefile > >> @@ -1037,6 +1037,7 @@ OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += > >> dump_extradata_bsf.o > >> 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_MP4TOANNEXB_BSF) += h264_mp4toannexb_bsf.o > >> OBJS-$(CONFIG_H264_REDUNDANT_PPS_BSF) += h264_redundant_pps_bsf.o > >> diff --git a/libavcodec/bitstream_filters.c > >> b/libavcodec/bitstream_filters.c index 338ef82..e1dc198 100644 > >> --- a/libavcodec/bitstream_filters.c > >> +++ b/libavcodec/bitstream_filters.c > >> @@ -29,6 +29,7 @@ extern const AVBitStreamFilter ff_chomp_bsf; > >> extern const AVBitStreamFilter ff_dump_extradata_bsf; extern const > >> AVBitStreamFilter ff_dca_core_bsf; extern const AVBitStreamFilter > >> ff_extract_extradata_bsf; > >> +extern const AVBitStreamFilter ff_filter_units_bsf; > >> extern const AVBitStreamFilter ff_h264_metadata_bsf; extern const > >> AVBitStreamFilter ff_h264_mp4toannexb_bsf; extern const > >> AVBitStreamFilter ff_h264_redundant_pps_bsf; diff --git > >> a/libavcodec/filter_units_bsf.c b/libavcodec/filter_units_bsf.c new > >> file mode 100644 index 0000000..3126f17 > >> --- /dev/null > >> +++ b/libavcodec/filter_units_bsf.c > >> @@ -0,0 +1,250 @@ > >> +/* > >> + * This file is part of FFmpeg. > >> + * > >> + * FFmpeg is free software; you can redistribute it and/or > >> + * modify it under the terms of the GNU Lesser General Public > >> + * License as published by the Free Software Foundation; either > >> + * version 2.1 of the License, or (at your option) any later version. > >> + * > >> + * FFmpeg is distributed in the hope that it will be useful, > >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >> + * Lesser General Public License for more details. > >> + * > >> + * You should have received a copy of the GNU Lesser General Public > >> + * License along with FFmpeg; if not, write to the Free Software > >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > >> +02110-1301 USA */ > >> + > >> +#include <stdlib.h> > >> + > >> +#include "libavutil/common.h" > >> +#include "libavutil/opt.h" > >> + > >> +#include "bsf.h" > >> +#include "cbs.h" > >> + > >> + > >> +typedef struct FilterUnitsContext { > >> + const AVClass *class; > >> + > >> + CodedBitstreamContext *cbc; > >> + > >> + const char *pass_types; > >> + const char *remove_types; > >> + > >> + int remove; > >> + CodedBitstreamUnitType *type_list; > >> + int nb_types; > >> +} FilterUnitsContext; > >> + > >> + > >> +static int filter_units_make_type_list(const char *list_string, > >> + CodedBitstreamUnitType **type_list, > >> + int *nb_types) { > >> + CodedBitstreamUnitType *list = NULL; > >> + int pass, count; > >> + > >> + for (pass = 1; pass <= 2; pass++) { > >> + long value, range_start, range_end; > >> + const char *str; > >> + char *value_end; > >> + > >> + count = 0; > >> + for (str = list_string; *str;) { > >> + value = strtol(str, &value_end, 0); > >> + if (str == value_end) > >> + goto invalid; > >> + str = (const char *)value_end; > >> + if (*str == '-') { > >> + ++str; > >> + range_start = value; > >> + range_end = strtol(str, &value_end, 0); > >> + if (str == value_end) > >> + goto invalid; > >> + > >> + for (value = range_start; value < range_end; value++) { > >> + if (pass == 2) > >> + list[count] = value; > >> + ++count; > >> + } > >> + } else { > >> + if (pass == 2) > >> + list[count] = value; > >> + ++count; > >> + } > >> + if (*str == '|') > >> + ++str; > >> + } > >> + if (pass == 1) { > >> + list = av_malloc_array(count, sizeof(*list)); > >> + if (!list) > >> + return AVERROR(ENOMEM); > >> + } > >> + } > >> + > >> + *type_list = list; > >> + *nb_types = count; > >> + return 0; > >> + > >> +invalid: > >> + av_freep(&list); > >> + return AVERROR(EINVAL); > >> +} > >> + > >> +static int filter_units_filter(AVBSFContext *bsf, AVPacket *out) { > >> + FilterUnitsContext *ctx = bsf->priv_data; > >> + AVPacket *in = NULL; > >> + CodedBitstreamFragment au; > >> + int err, i, j; > >> + > >> + while (1) { > >> + err = ff_bsf_get_packet(bsf, &in); > >> + if (err < 0) > >> + return err; > >> + > >> + err = ff_cbs_read_packet(ctx->cbc, &au, in); > >> + if (err < 0) { > >> + av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n"); > >> + goto fail; > >> + } > >> + > >> + for (i = 0; i < au.nb_units; i++) { > >> + for (j = 0; j < ctx->nb_types; j++) { > >> + if (au.units[i].type == ctx->type_list[j]) > >> + break; > >> + } > >> + if (ctx->remove ? j < ctx->nb_types > >> + : j >= ctx->nb_types) { > >> + ff_cbs_delete_unit(ctx->cbc, &au, i); > >> + --i; > >> + } > >> + } > >> + > >> + if (au.nb_units > 0) > >> + break; > >> + > >> + // Don't return packets with nothing in them. > >> + av_packet_free(&in); > >> + ff_cbs_fragment_uninit(ctx->cbc, &au); > >> + } > >> + > >> + err = ff_cbs_write_packet(ctx->cbc, out, &au); > >> + if (err < 0) { > >> + av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n"); > >> + goto fail; > >> + } > >> + > >> + err = av_packet_copy_props(out, in); > >> + if (err < 0) > >> + goto fail; > >> + > >> + err = 0; > > > > Unnecessary. av_packet_copy_props() already sets it to zero on success. > > I'm not entirely sure it's clearer, but ok. > > >> +fail: > >> + ff_cbs_fragment_uninit(ctx->cbc, &au); > >> + > >> + av_packet_free(&in); > >> + > >> + return err; > >> +} > >> + > >> +static int filter_units_init(AVBSFContext *bsf) { > >> + FilterUnitsContext *ctx = bsf->priv_data; > >> + int err; > >> + > >> + if (!(!ctx->pass_types ^ !ctx->remove_types)) { > >> + av_log(bsf, AV_LOG_ERROR, "Exactly one of pass_types or " > >> + "remove_types is required.\n"); > >> + return AVERROR(EINVAL); > >> + } > >> + > >> + if (ctx->pass_types) { > >> + ctx->remove = 0; > >> + err = filter_units_make_type_list(ctx->pass_types, > >> + &ctx->type_list, > >> &ctx->nb_types); > >> + if (err < 0) { > >> + av_log(bsf, AV_LOG_ERROR, "Failed to parse pass_types.\n"); > >> + return AVERROR(EINVAL); > > > > return err. It can also be ENOMEM. > > Fixed. > > >> + } > >> + } else { > >> + ctx->remove = 1; > >> + err = filter_units_make_type_list(ctx->remove_types, > >> + &ctx->type_list, > >> &ctx->nb_types); > >> + if (err < 0) { > >> + av_log(bsf, AV_LOG_ERROR, "Failed to parse remove_types.\n"); > >> + return AVERROR(EINVAL); > > > > Same. > > Same. > > >> + } > >> + } > >> + > >> + err = ff_cbs_init(&ctx->cbc, bsf->par_in->codec_id, bsf); > >> + if (err < 0) > >> + return err; > >> + > >> + // Don't actually decompose anything, we only want the unit data. > >> + ctx->cbc->decompose_unit_types = ctx->type_list; > >> + ctx->cbc->nb_decompose_unit_types = 0; > >> + > >> + if (bsf->par_in->extradata) { > >> + CodedBitstreamFragment ps; > >> + > >> + err = ff_cbs_read_extradata(ctx->cbc, &ps, bsf->par_in); > >> + if (err < 0) { > >> + av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n"); > >> + } else { > >> + err = ff_cbs_write_extradata(ctx->cbc, bsf->par_out, &ps); > >> + if (err < 0) > >> + av_log(bsf, AV_LOG_ERROR, "Failed to write extradata.\n"); > >> + } > >> + > >> + ff_cbs_fragment_uninit(ctx->cbc, &ps); > >> + } else { > >> + err = 0; > > > > Also Unnecessary. It's already zero from ff_cbs_init() above. > > Right. > > >> + } > >> + > >> + return err; > >> +} > >> + > >> +static void filter_units_close(AVBSFContext *bsf) { > >> + FilterUnitsContext *ctx = bsf->priv_data; > >> + > >> + av_freep(&ctx->type_list); > >> + > >> + ff_cbs_close(&ctx->cbc); > >> +} > >> + > >> +#define OFFSET(x) offsetof(FilterUnitsContext, x) static const > >> +AVOption filter_units_options[] = { > >> + { "pass_types", "List of unit types to pass through the filter.", > >> + OFFSET(pass_types), AV_OPT_TYPE_STRING, { .str = NULL } }, > >> + { "remove_types", "List of unit types to remove in the filter.", > >> + OFFSET(remove_types), AV_OPT_TYPE_STRING, { .str = NULL } }, > >> + > >> + { NULL } > >> +}; > >> + > >> +static const AVClass filter_units_class = { > >> + .class_name = "filter_units_bsf", > > > > Nit: Maybe just "filter_units". > > Sure. > > >> + .item_name = av_default_item_name, > >> + .option = filter_units_options, > >> + .version = LIBAVUTIL_VERSION_INT, > >> +}; > >> + > >> +static const enum AVCodecID filter_units_codec_ids[] = { > >> + AV_CODEC_ID_H264, > >> + AV_CODEC_ID_HEVC, > >> + AV_CODEC_ID_NONE, > >> +}; > >> + > >> +const AVBitStreamFilter ff_filter_units_bsf = { > >> + .name = "filter_units", > >> + .priv_data_size = sizeof(FilterUnitsContext), > >> + .priv_class = &filter_units_class, > >> + .init = &filter_units_init, > >> + .close = &filter_units_close, > >> + .filter = &filter_units_filter, > > > > The & is unnecessary for at least the last three. > > > >> + .codec_ids = filter_units_codec_ids, > >> +}; > >> > > > > What about a "passthrough" case, when neither pass_types or > > remove_types are provided, instead of erroring out? It's the standard > > behavior among most if not all bsfs (h264_mp4toannexb, vp9_superframe, > > aac_adtstoasc, etc). > > Imagine a batch process calling ffmpeg with this filter and each time > > with different input files and arguments. If for some file there's no > > need to remove anything, erroring out would break such a process. > > > > Also, doing av_packet_move_ref(out, in) for this, like in other > > filters, will be much faster than going through the entire cbs process > > even if the result is a noop. > > Ok, added. > > (Note that move_ref still isn't possible in general for packets which appear > to be unmodified, because the encapsulation might be different - e.g. > currently CBS always writes Annex B for H.26[45], so if the input is [AH]VCC > it still needs to be rewritten. That's also why the extradata gets rewritten > rather than just read.) > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Thanks Mark Thompson! (and Hendrik Leppkes :)) We tested this patch now and it solves our original issue, would love to see this getting merged. Eran _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel