[FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter
Mark Thompson
sw at jkqxz.net
Sun Mar 25 21:06:46 EEST 2018
On 25/03/18 13:40, Eran Kornblau wrote:
>> -----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
>>
>
> 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
More information about the ffmpeg-devel
mailing list