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
