[FFmpeg-devel] [PATCH v2 1/6] avformat: only allow a single bitstream filter when muxing
James Almer
jamrial at gmail.com
Mon Apr 27 01:35:35 EEST 2020
On 4/25/2020 5:14 AM, Marton Balint wrote:
>
>
> On Sun, 19 Apr 2020, Andreas Rheinhardt wrote:
>
>> 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.
>
> OK, changed locally.
>
>>
>>>
>>> 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.
>
> Will apply soon. Also ping for other patches in the series.
If you send a v3 of what's left of this set, please do it in a new
thread and not as replies to this one.
More information about the ffmpeg-devel
mailing list