[FFmpeg-devel] [PATCH v2 1/6] avformat: only allow a single bitstream filter when muxing
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sun Apr 19 03:16:11 EEST 2020
Marton Balint:
> Current muxers only use a single bitstream filter, so there is no need to
> maintain code which operates on a list of bitstream filters. When multiple
> bitstream filters are needed muxers can simply use a list bitstream filter.
>
> If there is a use case in the future when different bitstream filters should be
> added at subsequent packets then a new API possibly involving reconfiguring the
> list bistream filter can be added knowing the exact requirements.
>
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
> libavformat/dashenc.c | 6 ++----
> libavformat/internal.h | 5 ++---
> libavformat/mux.c | 6 +++---
> libavformat/segment.c | 6 ++----
> libavformat/utils.c | 27 +++++++++------------------
> 5 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 5a8cff4034..b977761a00 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -2307,10 +2307,8 @@ static int dash_check_bitstream(struct AVFormatContext *s, const AVPacket *avpkt
> if (ret == 1) {
> AVStream *st = s->streams[avpkt->stream_index];
> AVStream *ost = oc->streams[0];
> - st->internal->bsfcs = ost->internal->bsfcs;
> - st->internal->nb_bsfcs = ost->internal->nb_bsfcs;
> - ost->internal->bsfcs = NULL;
> - ost->internal->nb_bsfcs = 0;
> + st->internal->bsfc = ost->internal->bsfc;
> + ost->internal->bsfc = NULL;
> }
> return ret;
> }
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 7e4284b217..cafb4a9686 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -152,12 +152,11 @@ struct AVStreamInternal {
> int reorder;
>
> /**
> - * bitstream filters to run on stream
> + * bitstream filter to run on stream
> * - encoding: Set by muxer using ff_stream_add_bitstream_filter
> * - decoding: unused
> */
> - AVBSFContext **bsfcs;
> - int nb_bsfcs;
> + AVBSFContext *bsfc;
>
> /**
> * Whether or not check_bitstream should still be run on each packet
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 3d63d59faf..5209c84f40 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -824,7 +824,7 @@ static int prepare_input_packet(AVFormatContext *s, AVPacket *pkt)
>
> static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) {
> AVStream *st = s->streams[pkt->stream_index];
> - int i, ret;
> + int ret;
>
> if (!(s->flags & AVFMT_FLAG_AUTO_BSF))
> return 1;
> @@ -838,8 +838,8 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) {
> }
> }
>
> - for (i = 0; i < st->internal->nb_bsfcs; i++) {
> - AVBSFContext *ctx = st->internal->bsfcs[i];
> + if (st->internal->bsfc) {
> + AVBSFContext *ctx = st->internal->bsfc;
> // TODO: when any bitstream filter requires flushing at EOF, we'll need to
> // flush each stream's BSF chain on write_trailer.
> if ((ret = av_bsf_send_packet(ctx, pkt)) < 0) {
> diff --git a/libavformat/segment.c b/libavformat/segment.c
> index 60b72b7d15..32c09827eb 100644
> --- a/libavformat/segment.c
> +++ b/libavformat/segment.c
> @@ -1034,10 +1034,8 @@ static int seg_check_bitstream(struct AVFormatContext *s, const AVPacket *pkt)
> if (ret == 1) {
> AVStream *st = s->streams[pkt->stream_index];
> AVStream *ost = oc->streams[pkt->stream_index];
> - st->internal->bsfcs = ost->internal->bsfcs;
> - st->internal->nb_bsfcs = ost->internal->nb_bsfcs;
> - ost->internal->bsfcs = NULL;
> - ost->internal->nb_bsfcs = 0;
> + st->internal->bsfc = ost->internal->bsfc;
> + ost->internal->bsfc = NULL;
> }
> return ret;
> }
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index a58e47fabc..eff73252ec 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4410,10 +4410,7 @@ static void free_stream(AVStream **pst)
>
> if (st->internal) {
> avcodec_free_context(&st->internal->avctx);
> - for (i = 0; i < st->internal->nb_bsfcs; i++) {
> - av_bsf_free(&st->internal->bsfcs[i]);
> - av_freep(&st->internal->bsfcs);
I can't believe it! This only works if there is only one bsf. So a real
list has indeed never been tested.
> - }
> + av_bsf_free(&st->internal->bsfc);
> av_freep(&st->internal->priv_pts);
> av_bsf_free(&st->internal->extract_extradata.bsf);
> av_packet_free(&st->internal->extract_extradata.pkt);
> @@ -5574,7 +5571,11 @@ int ff_stream_add_bitstream_filter(AVStream *st, const char *name, const char *a
> int ret;
> const AVBitStreamFilter *bsf;
> AVBSFContext *bsfc;
> - AVCodecParameters *in_par;
> +
> + if (st->internal->bsfc) {
> + av_log(NULL, AV_LOG_ERROR, "A bitstream filter is already specified for stream %d\n", st->index);
> + return AVERROR(EINVAL);
> + }
Given that this is a situation which should not happen at all (until we
really have a situation where one wants to add more than one bsf, in
which case further changes to this function are required anyway), an
assert seems more appropriate.
>
> if (!(bsf = av_bsf_get_by_name(name))) {
> av_log(NULL, AV_LOG_ERROR, "Unknown bitstream filter '%s'\n", name);
> @@ -5584,15 +5585,8 @@ int ff_stream_add_bitstream_filter(AVStream *st, const char *name, const char *a
> if ((ret = av_bsf_alloc(bsf, &bsfc)) < 0)
> return ret;
>
> - if (st->internal->nb_bsfcs) {
> - in_par = st->internal->bsfcs[st->internal->nb_bsfcs - 1]->par_out;
> - bsfc->time_base_in = st->internal->bsfcs[st->internal->nb_bsfcs - 1]->time_base_out;
> - } else {
> - in_par = st->codecpar;
> - bsfc->time_base_in = st->time_base;
> - }
> -
> - if ((ret = avcodec_parameters_copy(bsfc->par_in, in_par)) < 0) {
> + bsfc->time_base_in = st->time_base;
> + if ((ret = avcodec_parameters_copy(bsfc->par_in, st->codecpar)) < 0) {
> av_bsf_free(&bsfc);
> return ret;
> }
> @@ -5615,10 +5609,7 @@ int ff_stream_add_bitstream_filter(AVStream *st, const char *name, const char *a
> return ret;
> }
>
> - if ((ret = av_dynarray_add_nofree(&st->internal->bsfcs, &st->internal->nb_bsfcs, bsfc))) {
> - av_bsf_free(&bsfc);
> - return ret;
> - }
> + st->internal->bsfc = bsfc;
>
> av_log(NULL, AV_LOG_VERBOSE,
> "Automatically inserted bitstream filter '%s'; args='%s'\n",
>
LGTM otherwise.
- Andreas
More information about the ffmpeg-devel
mailing list