On 25/03/18 13:40, Eran Kornblau wrote: >> -----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 >> > > Thanks Mark Thompson! (and Hendrik Leppkes :)) > We tested this patch now and it solves our original issue, would love to see > this getting merged.
Already in :) <http://git.videolan.org/?p=ffmpeg.git;a=commit;h=c99f837ddecad977018fd4d737c6070d167521c4> - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
