[FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter
Eran Kornblau
eran.kornblau at kaltura.com
Sun Mar 25 15:40:22 EEST 2018
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Mark Thompson
> Sent: Sunday, March 11, 2018 8:38 PM
> To: ffmpeg-devel at 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.kornblau at 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 at 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
More information about the ffmpeg-devel
mailing list