| Message ID | 20200428173725.12482-4-cus@passwd.hu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,v4,1/8] avformat/mux: move interleaved packet functions upwards | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Marton Balint: > Previously only 1:1 bitstream filters were supported, the end of the stream was > not signalled to the bitstream filters and time base changes were ignored. > > This change also allows muxers to set up bitstream filters regardless of the > autobsf flag during write_header instead of during check_bitstream and those > bitstream filters will always be executed. Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf when the user explicitly didn't want this. - Andreas > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/mux.c | 87 +++++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 56 insertions(+), 31 deletions(-) > > diff --git a/libavformat/mux.c b/libavformat/mux.c > index 33aed9da90..d3a98499ce 100644 > --- a/libavformat/mux.c > +++ b/libavformat/mux.c > @@ -1077,8 +1077,8 @@ static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket *in, in > return ff_interleave_packet_per_dts(s, out, in, flush); > } > > -static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { > - AVStream *st = s->streams[pkt->stream_index]; > +static int check_bitstream(AVFormatContext *s, AVStream *st, AVPacket *pkt) > +{ > int ret; > > if (!(s->flags & AVFMT_FLAG_AUTO_BSF)) > @@ -1093,30 +1093,6 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { > } > } > > - 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) { > - av_log(ctx, AV_LOG_ERROR, > - "Failed to send packet to filter %s for stream %d\n", > - ctx->filter->name, pkt->stream_index); > - return ret; > - } > - // TODO: when any automatically-added bitstream filter is generating multiple > - // output packets for a single input one, we'll need to call this in a loop > - // and write each output packet. > - if ((ret = av_bsf_receive_packet(ctx, pkt)) < 0) { > - if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) > - return 0; > - av_log(ctx, AV_LOG_ERROR, > - "Failed to receive packet from filter %s for stream %d\n", > - ctx->filter->name, pkt->stream_index); > - if (s->error_recognition & AV_EF_EXPLODE) > - return ret; > - return 0; > - } > - } > return 1; > } > > @@ -1161,17 +1137,53 @@ static int write_packet_common(AVFormatContext *s, AVStream *st, AVPacket *pkt, > } > } > > +static int write_packets_from_bsfs(AVFormatContext *s, AVStream *st, AVPacket *pkt, int interleaved) > +{ > + AVBSFContext *bsfc = st->internal->bsfc; > + int ret; > + > + if ((ret = av_bsf_send_packet(bsfc, pkt)) < 0) { > + av_log(s, AV_LOG_ERROR, > + "Failed to send packet to filter %s for stream %d\n", > + bsfc->filter->name, st->index); > + return ret; > + } > + > + do { > + ret = av_bsf_receive_packet(bsfc, pkt); > + if (ret < 0) { > + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) > + return 0; > + av_log(s, AV_LOG_ERROR, "Error applying bitstream filters to an output " > + "packet for stream #%d: %s\n", st->index, av_err2str(ret)); > + if (!(s->error_recognition & AV_EF_EXPLODE) && ret != AVERROR(ENOMEM)) > + continue; > + return ret; > + } > + av_packet_rescale_ts(pkt, bsfc->time_base_out, st->time_base); > + ret = write_packet_common(s, st, pkt, interleaved); > + if (ret >= 0 && !interleaved) // a successful write_packet_common already unrefed pkt for interleaved > + av_packet_unref(pkt); > + } while (ret >= 0); > + > + return ret; > +} > + > static int write_packets_common(AVFormatContext *s, AVStream *st, AVPacket *pkt, int interleaved) > { > int ret = prepare_input_packet(s, pkt); > if (ret < 0) > return ret; > > - ret = do_packet_auto_bsf(s, pkt); > - if (ret <= 0) > + ret = check_bitstream(s, st, pkt); > + if (ret < 0) > return ret; > > - return write_packet_common(s, st, pkt, interleaved); > + if (st->internal->bsfc) { > + return write_packets_from_bsfs(s, st, pkt, interleaved); > + } else { > + return write_packet_common(s, st, pkt, interleaved); > + } > } > > int av_write_frame(AVFormatContext *s, AVPacket *in) > @@ -1238,9 +1250,22 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) > > int av_write_trailer(AVFormatContext *s) > { > - int ret, i; > + int i, ret1, ret = 0; > + AVPacket pkt = {0}; > + av_init_packet(&pkt); > > - ret = interleaved_write_packet(s, NULL, 1); > + for (i = 0; i < s->nb_streams; i++) { > + if (s->streams[i]->internal->bsfc) { > + ret1 = write_packets_from_bsfs(s, s->streams[i], &pkt, 1/*interleaved*/); > + if (ret1 < 0) > + av_packet_unref(&pkt); > + if (ret >= 0) > + ret = ret1; > + } > + } > + ret1 = interleaved_write_packet(s, NULL, 1); > + if (ret >= 0) > + ret = ret1; > > if (s->oformat->write_trailer) { > if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) >
On Tue, 5 May 2020, Andreas Rheinhardt wrote: > Marton Balint: >> Previously only 1:1 bitstream filters were supported, the end of the stream was >> not signalled to the bitstream filters and time base changes were ignored. >> >> This change also allows muxers to set up bitstream filters regardless of the >> autobsf flag during write_header instead of during check_bitstream and those >> bitstream filters will always be executed. > > Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf > when the user explicitly didn't want this. > The user should not be allowed to create broken files, and the only way to achieve that is to force the BSF. I don't see a use case for disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my head I can't see a use case for disabling automatically inserted BSF-s in other muxers). So if you feel strongly about this and we still want to avoid creating corrupt files then the BSFs should be processed in mxf_interleave and gxf_interleave separately from the auto inserted bitstream filters. Do you think that is better? I don't think so. Regards, Marton > - Andreas > >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavformat/mux.c | 87 +++++++++++++++++++++++++++++++++++-------------------- >> 1 file changed, 56 insertions(+), 31 deletions(-) >> >> diff --git a/libavformat/mux.c b/libavformat/mux.c >> index 33aed9da90..d3a98499ce 100644 >> --- a/libavformat/mux.c >> +++ b/libavformat/mux.c >> @@ -1077,8 +1077,8 @@ static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket *in, in >> return ff_interleave_packet_per_dts(s, out, in, flush); >> } >> >> -static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { >> - AVStream *st = s->streams[pkt->stream_index]; >> +static int check_bitstream(AVFormatContext *s, AVStream *st, AVPacket *pkt) >> +{ >> int ret; >> >> if (!(s->flags & AVFMT_FLAG_AUTO_BSF)) >> @@ -1093,30 +1093,6 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { >> } >> } >> >> - 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) { >> - av_log(ctx, AV_LOG_ERROR, >> - "Failed to send packet to filter %s for stream %d\n", >> - ctx->filter->name, pkt->stream_index); >> - return ret; >> - } >> - // TODO: when any automatically-added bitstream filter is generating multiple >> - // output packets for a single input one, we'll need to call this in a loop >> - // and write each output packet. >> - if ((ret = av_bsf_receive_packet(ctx, pkt)) < 0) { >> - if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) >> - return 0; >> - av_log(ctx, AV_LOG_ERROR, >> - "Failed to receive packet from filter %s for stream %d\n", >> - ctx->filter->name, pkt->stream_index); >> - if (s->error_recognition & AV_EF_EXPLODE) >> - return ret; >> - return 0; >> - } >> - } >> return 1; >> } >> >> @@ -1161,17 +1137,53 @@ static int write_packet_common(AVFormatContext *s, AVStream *st, AVPacket *pkt, >> } >> } >> >> +static int write_packets_from_bsfs(AVFormatContext *s, AVStream *st, AVPacket *pkt, int interleaved) >> +{ >> + AVBSFContext *bsfc = st->internal->bsfc; >> + int ret; >> + >> + if ((ret = av_bsf_send_packet(bsfc, pkt)) < 0) { >> + av_log(s, AV_LOG_ERROR, >> + "Failed to send packet to filter %s for stream %d\n", >> + bsfc->filter->name, st->index); >> + return ret; >> + } >> + >> + do { >> + ret = av_bsf_receive_packet(bsfc, pkt); >> + if (ret < 0) { >> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) >> + return 0; >> + av_log(s, AV_LOG_ERROR, "Error applying bitstream filters to an output " >> + "packet for stream #%d: %s\n", st->index, av_err2str(ret)); >> + if (!(s->error_recognition & AV_EF_EXPLODE) && ret != AVERROR(ENOMEM)) >> + continue; >> + return ret; >> + } >> + av_packet_rescale_ts(pkt, bsfc->time_base_out, st->time_base); >> + ret = write_packet_common(s, st, pkt, interleaved); >> + if (ret >= 0 && !interleaved) // a successful write_packet_common already unrefed pkt for interleaved >> + av_packet_unref(pkt); >> + } while (ret >= 0); >> + >> + return ret; >> +} >> + >> static int write_packets_common(AVFormatContext *s, AVStream *st, AVPacket *pkt, int interleaved) >> { >> int ret = prepare_input_packet(s, pkt); >> if (ret < 0) >> return ret; >> >> - ret = do_packet_auto_bsf(s, pkt); >> - if (ret <= 0) >> + ret = check_bitstream(s, st, pkt); >> + if (ret < 0) >> return ret; >> >> - return write_packet_common(s, st, pkt, interleaved); >> + if (st->internal->bsfc) { >> + return write_packets_from_bsfs(s, st, pkt, interleaved); >> + } else { >> + return write_packet_common(s, st, pkt, interleaved); >> + } >> } >> >> int av_write_frame(AVFormatContext *s, AVPacket *in) >> @@ -1238,9 +1250,22 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) >> >> int av_write_trailer(AVFormatContext *s) >> { >> - int ret, i; >> + int i, ret1, ret = 0; >> + AVPacket pkt = {0}; >> + av_init_packet(&pkt); >> >> - ret = interleaved_write_packet(s, NULL, 1); >> + for (i = 0; i < s->nb_streams; i++) { >> + if (s->streams[i]->internal->bsfc) { >> + ret1 = write_packets_from_bsfs(s, s->streams[i], &pkt, 1/*interleaved*/); >> + if (ret1 < 0) >> + av_packet_unref(&pkt); >> + if (ret >= 0) >> + ret = ret1; >> + } >> + } >> + ret1 = interleaved_write_packet(s, NULL, 1); >> + if (ret >= 0) >> + ret = ret1; >> >> if (s->oformat->write_trailer) { >> if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint: > > > On Tue, 5 May 2020, Andreas Rheinhardt wrote: > >> Marton Balint: >>> Previously only 1:1 bitstream filters were supported, the end of the >>> stream was >>> not signalled to the bitstream filters and time base changes were >>> ignored. >>> >>> This change also allows muxers to set up bitstream filters regardless >>> of the >>> autobsf flag during write_header instead of during check_bitstream >>> and those >>> bitstream filters will always be executed. >> >> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >> when the user explicitly didn't want this. >> > > The user should not be allowed to create broken files, and the only way > to achieve that is to force the BSF. No, one could also check in the muxer that the packets are ok (these checks could be disabled if the bsf is enabled, of course) and error out if it is not. > I don't see a use case for > disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my > head I can't see a use case for disabling automatically inserted BSF-s > in other muxers). > A user who already knows that his packets are ok can do so to avoid the (admittedly small) overhead of these bsfs. (And even if you think that the option should be removed/should be made non-public (for internal testing only), then this does not change that this option exists and removing it would require the usual deprecation period (and consensus from other devs, of course; I personally don't have formed an opinion about this yet).) One use-case is if a user runs the pcm_rechunk bsf filter manually and then sends the output to both a framecrc as well as the mxf muxers in order to be able to test more thoroughly whether any write errors happened. Or where you want to write the same audio to multiple mxf muxers, all with the same audio settings. > So if you feel strongly about this and we still want to avoid creating > corrupt files then the BSFs should be processed in mxf_interleave and > gxf_interleave separately from the auto inserted bitstream filters. Do > you think that is better? I don't think so. > That would just be a worse form of automatically inserted bsf (and it would not work for av_write_frame()). - Andreas
On Tue, 5 May 2020, Andreas Rheinhardt wrote: > Marton Balint: >> >> >> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >> >>> Marton Balint: >>>> Previously only 1:1 bitstream filters were supported, the end of the >>>> stream was >>>> not signalled to the bitstream filters and time base changes were >>>> ignored. >>>> >>>> This change also allows muxers to set up bitstream filters regardless >>>> of the >>>> autobsf flag during write_header instead of during check_bitstream >>>> and those >>>> bitstream filters will always be executed. >>> >>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >>> when the user explicitly didn't want this. >>> >> >> The user should not be allowed to create broken files, and the only way >> to achieve that is to force the BSF. > > No, one could also check in the muxer that the packets are ok (these > checks could be disabled if the bsf is enabled, of course) and error out > if it is not. And that would duplicate some functionaltiy for negigable performance gain of not always doing a BSF. > >> I don't see a use case for >> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >> head I can't see a use case for disabling automatically inserted BSF-s >> in other muxers). >> > A user who already knows that his packets are ok can do so to avoid the > (admittedly small) overhead of these bsfs. (And even if you think that > the option should be removed/should be made non-public (for internal > testing only), then this does not change that this option exists and > removing it would require the usual deprecation period (and consensus > from other devs, of course; I personally don't have formed an opinion > about this yet).) With my patch the autobsf option is still honored for all existing bitstream filters which are inserted mid-stream (in check_bitstream). Only the ones which are inserted at write_header(), so in mxfenc and gxfenc are forced. > One use-case is if a user runs the pcm_rechunk bsf filter manually and > then sends the output to both a framecrc as well as the mxf muxers in > order to be able to test more thoroughly whether any write errors > happened. Or where you want to write the same audio to multiple mxf > muxers, all with the same audio settings. And for them the additional BSF which is auto inserted will only pass on their packets, it will only *check* for correct packets, exactly what you suggested. I cannot accept the performance gain as argument. > >> So if you feel strongly about this and we still want to avoid creating >> corrupt files then the BSFs should be processed in mxf_interleave and >> gxf_interleave separately from the auto inserted bitstream filters. Do >> you think that is better? I don't think so. >> > That would just be a worse form of automatically inserted bsf (and it > would not work for av_write_frame()). Please suggest something reasonable to follow up on this. Thanks, Marton
Marton Balint: > > > On Tue, 5 May 2020, Andreas Rheinhardt wrote: > >> Marton Balint: >>> >>> >>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>> >>>> Marton Balint: >>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>> stream was >>>>> not signalled to the bitstream filters and time base changes were >>>>> ignored. >>>>> >>>>> This change also allows muxers to set up bitstream filters regardless >>>>> of the >>>>> autobsf flag during write_header instead of during check_bitstream >>>>> and those >>>>> bitstream filters will always be executed. >>>> >>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >>>> when the user explicitly didn't want this. >>>> >>> >>> The user should not be allowed to create broken files, and the only way >>> to achieve that is to force the BSF. >> >> No, one could also check in the muxer that the packets are ok (these >> checks could be disabled if the bsf is enabled, of course) and error out >> if it is not. > > And that would duplicate some functionaltiy for negigable performance > gain of not always doing a BSF. > >> >>> I don't see a use case for >>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >>> head I can't see a use case for disabling automatically inserted BSF-s >>> in other muxers). >>> >> A user who already knows that his packets are ok can do so to avoid the >> (admittedly small) overhead of these bsfs. (And even if you think that >> the option should be removed/should be made non-public (for internal >> testing only), then this does not change that this option exists and >> removing it would require the usual deprecation period (and consensus >> from other devs, of course; I personally don't have formed an opinion >> about this yet).) > > With my patch the autobsf option is still honored for all existing > bitstream filters which are inserted mid-stream (in check_bitstream). > Only the ones which are inserted at write_header(), so in mxfenc and > gxfenc are forced. > >> One use-case is if a user runs the pcm_rechunk bsf filter manually and >> then sends the output to both a framecrc as well as the mxf muxers in >> order to be able to test more thoroughly whether any write errors >> happened. Or where you want to write the same audio to multiple mxf >> muxers, all with the same audio settings. > > And for them the additional BSF which is auto inserted will only pass on > their packets, it will only *check* for correct packets, exactly what > you suggested. I cannot accept the performance gain as argument. > >> >>> So if you feel strongly about this and we still want to avoid creating >>> corrupt files then the BSFs should be processed in mxf_interleave and >>> gxf_interleave separately from the auto inserted bitstream filters. Do >>> you think that is better? I don't think so. >>> >> That would just be a worse form of automatically inserted bsf (and it >> would not work for av_write_frame()). > > Please suggest something reasonable to follow up on this. > How about: You error out in write_header if there is an audio track and the user does not want automatically inserted bitstream filters? - Andreas
On Tue, 5 May 2020, Andreas Rheinhardt wrote: > Marton Balint: >> >> >> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >> >>> Marton Balint: >>>> >>>> >>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>>> >>>>> Marton Balint: >>>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>>> stream was >>>>>> not signalled to the bitstream filters and time base changes were >>>>>> ignored. >>>>>> >>>>>> This change also allows muxers to set up bitstream filters regardless >>>>>> of the >>>>>> autobsf flag during write_header instead of during check_bitstream >>>>>> and those >>>>>> bitstream filters will always be executed. >>>>> >>>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >>>>> when the user explicitly didn't want this. >>>>> >>>> >>>> The user should not be allowed to create broken files, and the only way >>>> to achieve that is to force the BSF. >>> >>> No, one could also check in the muxer that the packets are ok (these >>> checks could be disabled if the bsf is enabled, of course) and error out >>> if it is not. >> >> And that would duplicate some functionaltiy for negigable performance >> gain of not always doing a BSF. >> >>> >>>> I don't see a use case for >>>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >>>> head I can't see a use case for disabling automatically inserted BSF-s >>>> in other muxers). >>>> >>> A user who already knows that his packets are ok can do so to avoid the >>> (admittedly small) overhead of these bsfs. (And even if you think that >>> the option should be removed/should be made non-public (for internal >>> testing only), then this does not change that this option exists and >>> removing it would require the usual deprecation period (and consensus >>> from other devs, of course; I personally don't have formed an opinion >>> about this yet).) >> >> With my patch the autobsf option is still honored for all existing >> bitstream filters which are inserted mid-stream (in check_bitstream). >> Only the ones which are inserted at write_header(), so in mxfenc and >> gxfenc are forced. >> >>> One use-case is if a user runs the pcm_rechunk bsf filter manually and >>> then sends the output to both a framecrc as well as the mxf muxers in >>> order to be able to test more thoroughly whether any write errors >>> happened. Or where you want to write the same audio to multiple mxf >>> muxers, all with the same audio settings. >> >> And for them the additional BSF which is auto inserted will only pass on >> their packets, it will only *check* for correct packets, exactly what >> you suggested. I cannot accept the performance gain as argument. >> >>> >>>> So if you feel strongly about this and we still want to avoid creating >>>> corrupt files then the BSFs should be processed in mxf_interleave and >>>> gxf_interleave separately from the auto inserted bitstream filters. Do >>>> you think that is better? I don't think so. >>>> >>> That would just be a worse form of automatically inserted bsf (and it >>> would not work for av_write_frame()). >> >> Please suggest something reasonable to follow up on this. >> > How about: You error out in write_header if there is an audio track and > the user does not want automatically inserted bitstream filters? Then maybe better to do it ff_stream_add_bitstream_filter? Thanks, Marton
Marton Balint: > > > On Tue, 5 May 2020, Andreas Rheinhardt wrote: > >> Marton Balint: >>> >>> >>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>> >>>> Marton Balint: >>>>> >>>>> >>>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>>>> >>>>>> Marton Balint: >>>>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>>>> stream was >>>>>>> not signalled to the bitstream filters and time base changes were >>>>>>> ignored. >>>>>>> >>>>>>> This change also allows muxers to set up bitstream filters >>>>>>> regardless >>>>>>> of the >>>>>>> autobsf flag during write_header instead of during check_bitstream >>>>>>> and those >>>>>>> bitstream filters will always be executed. >>>>>> >>>>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add >>>>>> a bsf >>>>>> when the user explicitly didn't want this. >>>>>> >>>>> >>>>> The user should not be allowed to create broken files, and the only >>>>> way >>>>> to achieve that is to force the BSF. >>>> >>>> No, one could also check in the muxer that the packets are ok (these >>>> checks could be disabled if the bsf is enabled, of course) and error >>>> out >>>> if it is not. >>> >>> And that would duplicate some functionaltiy for negigable performance >>> gain of not always doing a BSF. >>> >>>> >>>>> I don't see a use case for >>>>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top >>>>> of my >>>>> head I can't see a use case for disabling automatically inserted BSF-s >>>>> in other muxers). >>>>> >>>> A user who already knows that his packets are ok can do so to avoid the >>>> (admittedly small) overhead of these bsfs. (And even if you think that >>>> the option should be removed/should be made non-public (for internal >>>> testing only), then this does not change that this option exists and >>>> removing it would require the usual deprecation period (and consensus >>>> from other devs, of course; I personally don't have formed an opinion >>>> about this yet).) >>> >>> With my patch the autobsf option is still honored for all existing >>> bitstream filters which are inserted mid-stream (in check_bitstream). >>> Only the ones which are inserted at write_header(), so in mxfenc and >>> gxfenc are forced. >>> >>>> One use-case is if a user runs the pcm_rechunk bsf filter manually and >>>> then sends the output to both a framecrc as well as the mxf muxers in >>>> order to be able to test more thoroughly whether any write errors >>>> happened. Or where you want to write the same audio to multiple mxf >>>> muxers, all with the same audio settings. >>> >>> And for them the additional BSF which is auto inserted will only pass on >>> their packets, it will only *check* for correct packets, exactly what >>> you suggested. I cannot accept the performance gain as argument. >>> >>>> >>>>> So if you feel strongly about this and we still want to avoid creating >>>>> corrupt files then the BSFs should be processed in mxf_interleave and >>>>> gxf_interleave separately from the auto inserted bitstream filters. Do >>>>> you think that is better? I don't think so. >>>>> >>>> That would just be a worse form of automatically inserted bsf (and it >>>> would not work for av_write_frame()). >>> >>> Please suggest something reasonable to follow up on this. >>> >> How about: You error out in write_header if there is an audio track and >> the user does not want automatically inserted bitstream filters? > > Then maybe better to do it ff_stream_add_bitstream_filter? > Yeah, that sounds like the right place for this. - Andreas
Marton Balint: > > > On Tue, 5 May 2020, Andreas Rheinhardt wrote: > >> Marton Balint: >>> Previously only 1:1 bitstream filters were supported, the end of the >>> stream was >>> not signalled to the bitstream filters and time base changes were >>> ignored. >>> >>> This change also allows muxers to set up bitstream filters regardless >>> of the >>> autobsf flag during write_header instead of during check_bitstream >>> and those >>> bitstream filters will always be executed. >> >> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >> when the user explicitly didn't want this. >> > > The user should not be allowed to create broken files, and the only way > to achieve that is to force the BSF. I don't see a use case for > disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my > head I can't see a use case for disabling automatically inserted BSF-s > in other muxers). > When automatic bitstream filtering was introduced in 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every muxer that potentially might include a bsf automatically (as indicated by the existence of the check_bitstream()-function) was delayed until the first packet would be written. This meant that there was a high probability of having a packet for the relevant stream when using the interleaved codepath. In particular, this meant that the Matroska muxer now received proper extradata for AAC when writing the header even before it received any packet with side data containing potential extradata at all. The reason was that the aac_adtstoasc bsf has simply overwritten the output extradata when dealing with the first packet (that was the old bsf API without init functions; when the new bsf API was merged, the merge commit (not the original commit) propagated the API violating behaviour). 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because of this delay: After all, the delay existed as soon as the AVOutputFormat had a check_bitstream function, even when no stream had a codec for which the check_bitstream function would add a bsf at all. As time passed, the API-violating behaviour of aac_adtstoasc was fixed and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing the header. The very same patchset containing said commit also contained a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to concerns what happens when the bsf is not available. (For the record, ff_stream_add_bitstream_filter() will error out if a bsf is not available and the packet discarded. If the caller sends another packet for this stream and a bsf that should be added isn't available the packet will be rejected as well and so on.) The fact that one is forced to include certain automatically inserted bitstream filters even when one knows that one will only use them in scenarios where they merely passthrough the packets is certainly a downside of eliminating this flag. But now that I have reread the history I am nevertheless in favour of deprecation. - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220993.html
On Tue, 5 May 2020, Andreas Rheinhardt wrote: > Marton Balint: >> >> >> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >> >>> Marton Balint: >>>> Previously only 1:1 bitstream filters were supported, the end of the >>>> stream was >>>> not signalled to the bitstream filters and time base changes were >>>> ignored. >>>> >>>> This change also allows muxers to set up bitstream filters regardless >>>> of the >>>> autobsf flag during write_header instead of during check_bitstream >>>> and those >>>> bitstream filters will always be executed. >>> >>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >>> when the user explicitly didn't want this. >>> >> >> The user should not be allowed to create broken files, and the only way >> to achieve that is to force the BSF. I don't see a use case for >> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >> head I can't see a use case for disabling automatically inserted BSF-s >> in other muxers). >> > When automatic bitstream filtering was introduced in > 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every > muxer that potentially might include a bsf automatically (as indicated > by the existence of the check_bitstream()-function) was delayed until > the first packet would be written. This meant that there was a high > probability of having a packet for the relevant stream when using the > interleaved codepath. > In particular, this meant that the Matroska muxer now received proper > extradata for AAC when writing the header even before it received any > packet with side data containing potential extradata at all. The reason > was that the aac_adtstoasc bsf has simply overwritten the output > extradata when dealing with the first packet (that was the old bsf API > without init functions; when the new bsf API was merged, the merge > commit (not the original commit) propagated the API violating behaviour). > 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because > of this delay: After all, the delay existed as soon as the > AVOutputFormat had a check_bitstream function, even when no stream had a > codec for which the check_bitstream function would add a bsf at all. > > As time passed, the API-violating behaviour of aac_adtstoasc was fixed > and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing > the header. The very same patchset containing said commit also contained > a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to > concerns what happens when the bsf is not available. > > (For the record, ff_stream_add_bitstream_filter() will error out if a > bsf is not available and the packet discarded. If the caller sends > another packet for this stream and a bsf that should be added isn't > available the packet will be rejected as well and so on.) > > The fact that one is forced to include certain automatically inserted > bitstream filters even when one knows that one will only use them in > scenarios where they merely passthrough the packets is certainly a > downside of eliminating this flag. But now that I have reread the > history I am nevertheless in favour of deprecation. Wow, thanks for checking this. But if we are moving in the direction of deprecation, then I'd rather not add the extra check to ff_stream_add_bitstream_filter because it will be removed later anyway. Also when I tried to add that, there was a fate failure caused by the segment muxer setting the -autobsf flag but calling the muxer's check_bitstream function directly. Or should I add create a patch which simply removes the -autobsf additional options from segment.c? Regards, Marton
On 5/5/2020 9:42 AM, Andreas Rheinhardt wrote: > As time passed, the API-violating behaviour of aac_adtstoasc was fixed > and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing > the header. The very same patchset containing said commit also contained > a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to > concerns what happens when the bsf is not available. This is solved by having the muxers select the required bsfs in configure. The IVF muxer already does it.
On Tue, 5 May 2020, Marton Balint wrote: > > > On Tue, 5 May 2020, Andreas Rheinhardt wrote: > >> Marton Balint: >>> >>> >>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>> >>>> Marton Balint: >>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>> stream was >>>>> not signalled to the bitstream filters and time base changes were >>>>> ignored. >>>>> >>>>> This change also allows muxers to set up bitstream filters regardless >>>>> of the >>>>> autobsf flag during write_header instead of during check_bitstream >>>>> and those >>>>> bitstream filters will always be executed. >>>> >>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a bsf >>>> when the user explicitly didn't want this. >>>> >>> >>> The user should not be allowed to create broken files, and the only way >>> to achieve that is to force the BSF. I don't see a use case for >>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >>> head I can't see a use case for disabling automatically inserted BSF-s >>> in other muxers). >>> >> When automatic bitstream filtering was introduced in >> 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every >> muxer that potentially might include a bsf automatically (as indicated >> by the existence of the check_bitstream()-function) was delayed until >> the first packet would be written. This meant that there was a high >> probability of having a packet for the relevant stream when using the >> interleaved codepath. >> In particular, this meant that the Matroska muxer now received proper >> extradata for AAC when writing the header even before it received any >> packet with side data containing potential extradata at all. The reason >> was that the aac_adtstoasc bsf has simply overwritten the output >> extradata when dealing with the first packet (that was the old bsf API >> without init functions; when the new bsf API was merged, the merge >> commit (not the original commit) propagated the API violating behaviour). >> 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because >> of this delay: After all, the delay existed as soon as the >> AVOutputFormat had a check_bitstream function, even when no stream had a >> codec for which the check_bitstream function would add a bsf at all. >> >> As time passed, the API-violating behaviour of aac_adtstoasc was fixed >> and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing >> the header. The very same patchset containing said commit also contained >> a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to >> concerns what happens when the bsf is not available. >> >> (For the record, ff_stream_add_bitstream_filter() will error out if a >> bsf is not available and the packet discarded. If the caller sends >> another packet for this stream and a bsf that should be added isn't >> available the packet will be rejected as well and so on.) >> >> The fact that one is forced to include certain automatically inserted >> bitstream filters even when one knows that one will only use them in >> scenarios where they merely passthrough the packets is certainly a >> downside of eliminating this flag. But now that I have reread the >> history I am nevertheless in favour of deprecation. > > Wow, thanks for checking this. But if we are moving in the direction of > deprecation, then I'd rather not add the extra check to > ff_stream_add_bitstream_filter because it will be removed later anyway. > > Also when I tried to add that, there was a fate failure caused by the > segment muxer setting the -autobsf flag but calling the muxer's > check_bitstream function directly. Or should I add create a patch which > simply removes the -autobsf additional options from segment.c? So considering that autobsf will probably be deprecated anyway can I apply the patch as is? (And finally the series?). Thanks, Marton
Marton Balint: > > > On Tue, 5 May 2020, Marton Balint wrote: > >> >> >> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >> >>> Marton Balint: >>>> >>>> >>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>>> >>>>> Marton Balint: >>>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>>> stream was >>>>>> not signalled to the bitstream filters and time base changes were >>>>>> ignored. >>>>>> >>>>>> This change also allows muxers to set up bitstream filters regardless >>>>>> of the >>>>>> autobsf flag during write_header instead of during check_bitstream >>>>>> and those >>>>>> bitstream filters will always be executed. >>>>> >>>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a >>>>> bsf >>>>> when the user explicitly didn't want this. >>>>> >>>> >>>> The user should not be allowed to create broken files, and the only way >>>> to achieve that is to force the BSF. I don't see a use case for >>>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >>>> head I can't see a use case for disabling automatically inserted BSF-s >>>> in other muxers). >>>> >>> When automatic bitstream filtering was introduced in >>> 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every >>> muxer that potentially might include a bsf automatically (as indicated >>> by the existence of the check_bitstream()-function) was delayed until >>> the first packet would be written. This meant that there was a high >>> probability of having a packet for the relevant stream when using the >>> interleaved codepath. >>> In particular, this meant that the Matroska muxer now received proper >>> extradata for AAC when writing the header even before it received any >>> packet with side data containing potential extradata at all. The reason >>> was that the aac_adtstoasc bsf has simply overwritten the output >>> extradata when dealing with the first packet (that was the old bsf API >>> without init functions; when the new bsf API was merged, the merge >>> commit (not the original commit) propagated the API violating >>> behaviour). >>> 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because >>> of this delay: After all, the delay existed as soon as the >>> AVOutputFormat had a check_bitstream function, even when no stream had a >>> codec for which the check_bitstream function would add a bsf at all. >>> >>> As time passed, the API-violating behaviour of aac_adtstoasc was fixed >>> and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing >>> the header. The very same patchset containing said commit also contained >>> a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to >>> concerns what happens when the bsf is not available. >>> >>> (For the record, ff_stream_add_bitstream_filter() will error out if a >>> bsf is not available and the packet discarded. If the caller sends >>> another packet for this stream and a bsf that should be added isn't >>> available the packet will be rejected as well and so on.) >>> >>> The fact that one is forced to include certain automatically inserted >>> bitstream filters even when one knows that one will only use them in >>> scenarios where they merely passthrough the packets is certainly a >>> downside of eliminating this flag. But now that I have reread the >>> history I am nevertheless in favour of deprecation. >> >> Wow, thanks for checking this. But if we are moving in the direction >> of deprecation, then I'd rather not add the extra check to >> ff_stream_add_bitstream_filter because it will be removed later anyway. >> >> Also when I tried to add that, there was a fate failure caused by the >> segment muxer setting the -autobsf flag but calling the muxer's >> check_bitstream function directly. Or should I add create a patch >> which simply removes the -autobsf additional options from segment.c? > > So considering that autobsf will probably be deprecated anyway can I > apply the patch as is? (And finally the series?). > According to me: yes. But there is one little detail that I just noticed: write_packets_common() has an AVStream parameter and both of its callers (av_write_frame() and av_interleaved_write_frame() essentially pass s->streams[pkt->stream_index] into it, but don't use it themselves. This parameter should be eliminated and replaced by a local variable in write_packets_common(). - Andreas
On Thu, 7 May 2020, Andreas Rheinhardt wrote: > Marton Balint: >> >> >> On Tue, 5 May 2020, Marton Balint wrote: >> >>> >>> >>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>> >>>> Marton Balint: >>>>> >>>>> >>>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>>>> >>>>>> Marton Balint: >>>>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>>>> stream was >>>>>>> not signalled to the bitstream filters and time base changes were >>>>>>> ignored. >>>>>>> >>>>>>> This change also allows muxers to set up bitstream filters regardless >>>>>>> of the >>>>>>> autobsf flag during write_header instead of during check_bitstream >>>>>>> and those >>>>>>> bitstream filters will always be executed. >>>>>> >>>>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a >>>>>> bsf >>>>>> when the user explicitly didn't want this. >>>>>> >>>>> >>>>> The user should not be allowed to create broken files, and the only way >>>>> to achieve that is to force the BSF. I don't see a use case for >>>>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >>>>> head I can't see a use case for disabling automatically inserted BSF-s >>>>> in other muxers). >>>>> >>>> When automatic bitstream filtering was introduced in >>>> 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every >>>> muxer that potentially might include a bsf automatically (as indicated >>>> by the existence of the check_bitstream()-function) was delayed until >>>> the first packet would be written. This meant that there was a high >>>> probability of having a packet for the relevant stream when using the >>>> interleaved codepath. >>>> In particular, this meant that the Matroska muxer now received proper >>>> extradata for AAC when writing the header even before it received any >>>> packet with side data containing potential extradata at all. The reason >>>> was that the aac_adtstoasc bsf has simply overwritten the output >>>> extradata when dealing with the first packet (that was the old bsf API >>>> without init functions; when the new bsf API was merged, the merge >>>> commit (not the original commit) propagated the API violating >>>> behaviour). >>>> 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because >>>> of this delay: After all, the delay existed as soon as the >>>> AVOutputFormat had a check_bitstream function, even when no stream had a >>>> codec for which the check_bitstream function would add a bsf at all. >>>> >>>> As time passed, the API-violating behaviour of aac_adtstoasc was fixed >>>> and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing >>>> the header. The very same patchset containing said commit also contained >>>> a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to >>>> concerns what happens when the bsf is not available. >>>> >>>> (For the record, ff_stream_add_bitstream_filter() will error out if a >>>> bsf is not available and the packet discarded. If the caller sends >>>> another packet for this stream and a bsf that should be added isn't >>>> available the packet will be rejected as well and so on.) >>>> >>>> The fact that one is forced to include certain automatically inserted >>>> bitstream filters even when one knows that one will only use them in >>>> scenarios where they merely passthrough the packets is certainly a >>>> downside of eliminating this flag. But now that I have reread the >>>> history I am nevertheless in favour of deprecation. >>> >>> Wow, thanks for checking this. But if we are moving in the direction >>> of deprecation, then I'd rather not add the extra check to >>> ff_stream_add_bitstream_filter because it will be removed later anyway. >>> >>> Also when I tried to add that, there was a fate failure caused by the >>> segment muxer setting the -autobsf flag but calling the muxer's >>> check_bitstream function directly. Or should I add create a patch >>> which simply removes the -autobsf additional options from segment.c? >> >> So considering that autobsf will probably be deprecated anyway can I >> apply the patch as is? (And finally the series?). >> > According to me: yes. But there is one little detail that I just > noticed: write_packets_common() has an AVStream parameter and both of > its callers (av_write_frame() and av_interleaved_write_frame() > essentially pass s->streams[pkt->stream_index] into it, but don't use it > themselves. This parameter should be eliminated and replaced by a local > variable in write_packets_common(). Thanks, applied the series with that fix. Regards, Marton
On 5/7/2020 3:51 PM, Marton Balint wrote: > > > On Tue, 5 May 2020, Marton Balint wrote: > >> >> >> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >> >>> Marton Balint: >>>> >>>> >>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote: >>>> >>>>> Marton Balint: >>>>>> Previously only 1:1 bitstream filters were supported, the end of the >>>>>> stream was >>>>>> not signalled to the bitstream filters and time base changes were >>>>>> ignored. >>>>>> >>>>>> This change also allows muxers to set up bitstream filters regardless >>>>>> of the >>>>>> autobsf flag during write_header instead of during check_bitstream >>>>>> and those >>>>>> bitstream filters will always be executed. >>>>> >>>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add a >>>>> bsf >>>>> when the user explicitly didn't want this. >>>>> >>>> >>>> The user should not be allowed to create broken files, and the only way >>>> to achieve that is to force the BSF. I don't see a use case for >>>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top of my >>>> head I can't see a use case for disabling automatically inserted BSF-s >>>> in other muxers). >>>> >>> When automatic bitstream filtering was introduced in >>> 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every >>> muxer that potentially might include a bsf automatically (as indicated >>> by the existence of the check_bitstream()-function) was delayed until >>> the first packet would be written. This meant that there was a high >>> probability of having a packet for the relevant stream when using the >>> interleaved codepath. >>> In particular, this meant that the Matroska muxer now received proper >>> extradata for AAC when writing the header even before it received any >>> packet with side data containing potential extradata at all. The reason >>> was that the aac_adtstoasc bsf has simply overwritten the output >>> extradata when dealing with the first packet (that was the old bsf API >>> without init functions; when the new bsf API was merged, the merge >>> commit (not the original commit) propagated the API violating >>> behaviour). >>> 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because >>> of this delay: After all, the delay existed as soon as the >>> AVOutputFormat had a check_bitstream function, even when no stream had a >>> codec for which the check_bitstream function would add a bsf at all. >>> >>> As time passed, the API-violating behaviour of aac_adtstoasc was fixed >>> and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing >>> the header. The very same patchset containing said commit also contained >>> a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to >>> concerns what happens when the bsf is not available. >>> >>> (For the record, ff_stream_add_bitstream_filter() will error out if a >>> bsf is not available and the packet discarded. If the caller sends >>> another packet for this stream and a bsf that should be added isn't >>> available the packet will be rejected as well and so on.) >>> >>> The fact that one is forced to include certain automatically inserted >>> bitstream filters even when one knows that one will only use them in >>> scenarios where they merely passthrough the packets is certainly a >>> downside of eliminating this flag. But now that I have reread the >>> history I am nevertheless in favour of deprecation. >> >> Wow, thanks for checking this. But if we are moving in the direction >> of deprecation, then I'd rather not add the extra check to >> ff_stream_add_bitstream_filter because it will be removed later anyway. >> >> Also when I tried to add that, there was a fate failure caused by the >> segment muxer setting the -autobsf flag but calling the muxer's >> check_bitstream function directly. Or should I add create a patch >> which simply removes the -autobsf additional options from segment.c? > > So considering that autobsf will probably be deprecated anyway can I > apply the patch as is? (And finally the series?). > > Thanks, > Marton It's fine by me. I'll resubmit the deprecation patch as well.
diff --git a/libavformat/mux.c b/libavformat/mux.c index 33aed9da90..d3a98499ce 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -1077,8 +1077,8 @@ static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket *in, in return ff_interleave_packet_per_dts(s, out, in, flush); } -static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { - AVStream *st = s->streams[pkt->stream_index]; +static int check_bitstream(AVFormatContext *s, AVStream *st, AVPacket *pkt) +{ int ret; if (!(s->flags & AVFMT_FLAG_AUTO_BSF)) @@ -1093,30 +1093,6 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) { } } - 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) { - av_log(ctx, AV_LOG_ERROR, - "Failed to send packet to filter %s for stream %d\n", - ctx->filter->name, pkt->stream_index); - return ret; - } - // TODO: when any automatically-added bitstream filter is generating multiple - // output packets for a single input one, we'll need to call this in a loop - // and write each output packet. - if ((ret = av_bsf_receive_packet(ctx, pkt)) < 0) { - if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) - return 0; - av_log(ctx, AV_LOG_ERROR, - "Failed to receive packet from filter %s for stream %d\n", - ctx->filter->name, pkt->stream_index); - if (s->error_recognition & AV_EF_EXPLODE) - return ret; - return 0; - } - } return 1; } @@ -1161,17 +1137,53 @@ static int write_packet_common(AVFormatContext *s, AVStream *st, AVPacket *pkt, } } +static int write_packets_from_bsfs(AVFormatContext *s, AVStream *st, AVPacket *pkt, int interleaved) +{ + AVBSFContext *bsfc = st->internal->bsfc; + int ret; + + if ((ret = av_bsf_send_packet(bsfc, pkt)) < 0) { + av_log(s, AV_LOG_ERROR, + "Failed to send packet to filter %s for stream %d\n", + bsfc->filter->name, st->index); + return ret; + } + + do { + ret = av_bsf_receive_packet(bsfc, pkt); + if (ret < 0) { + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) + return 0; + av_log(s, AV_LOG_ERROR, "Error applying bitstream filters to an output " + "packet for stream #%d: %s\n", st->index, av_err2str(ret)); + if (!(s->error_recognition & AV_EF_EXPLODE) && ret != AVERROR(ENOMEM)) + continue; + return ret; + } + av_packet_rescale_ts(pkt, bsfc->time_base_out, st->time_base); + ret = write_packet_common(s, st, pkt, interleaved); + if (ret >= 0 && !interleaved) // a successful write_packet_common already unrefed pkt for interleaved + av_packet_unref(pkt); + } while (ret >= 0); + + return ret; +} + static int write_packets_common(AVFormatContext *s, AVStream *st, AVPacket *pkt, int interleaved) { int ret = prepare_input_packet(s, pkt); if (ret < 0) return ret; - ret = do_packet_auto_bsf(s, pkt); - if (ret <= 0) + ret = check_bitstream(s, st, pkt); + if (ret < 0) return ret; - return write_packet_common(s, st, pkt, interleaved); + if (st->internal->bsfc) { + return write_packets_from_bsfs(s, st, pkt, interleaved); + } else { + return write_packet_common(s, st, pkt, interleaved); + } } int av_write_frame(AVFormatContext *s, AVPacket *in) @@ -1238,9 +1250,22 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt) int av_write_trailer(AVFormatContext *s) { - int ret, i; + int i, ret1, ret = 0; + AVPacket pkt = {0}; + av_init_packet(&pkt); - ret = interleaved_write_packet(s, NULL, 1); + for (i = 0; i < s->nb_streams; i++) { + if (s->streams[i]->internal->bsfc) { + ret1 = write_packets_from_bsfs(s, s->streams[i], &pkt, 1/*interleaved*/); + if (ret1 < 0) + av_packet_unref(&pkt); + if (ret >= 0) + ret = ret1; + } + } + ret1 = interleaved_write_packet(s, NULL, 1); + if (ret >= 0) + ret = ret1; if (s->oformat->write_trailer) { if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb)
Previously only 1:1 bitstream filters were supported, the end of the stream was not signalled to the bitstream filters and time base changes were ignored. This change also allows muxers to set up bitstream filters regardless of the autobsf flag during write_header instead of during check_bitstream and those bitstream filters will always be executed. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/mux.c | 87 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 31 deletions(-)