[FFmpeg-devel] [PATCH] lavc: Add filter_units bitstream filter

Mark Thompson sw at jkqxz.net
Sun Mar 11 20:37:44 EET 2018


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


More information about the ffmpeg-devel mailing list