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.
>
> Signed-off-by: Marton Balint <c...@passwd.hu>
> ---
> libavformat/mux.c | 91
> ++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 57 insertions(+), 34 deletions(-)
>
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 4118d221e0..c2b6d4461e 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -822,14 +822,13 @@ static int prepare_input_packet(AVFormatContext *s,
> AVPacket *pkt)
> return 0;
> }
>
> -static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) {
> - AVStream *st = s->streams[pkt->stream_index];
> +static int need_auto_bsf(AVFormatContext *s, AVStream *st, AVPacket *pkt) {
{ belongs in a line of its own.
> int ret;
>
> if (!(s->flags & AVFMT_FLAG_AUTO_BSF))
> - return 1;
> + return 0;
>
> - if (s->oformat->check_bitstream) {
> + if (pkt && s->oformat->check_bitstream) {
> if (!st->internal->bitstream_checked) {
> if ((ret = s->oformat->check_bitstream(s, pkt)) < 0)
> return ret;
> @@ -838,31 +837,7 @@ 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;
> + return !!st->internal->bsfc;
> }
>
> static int interleaved_write_packet(AVFormatContext *s, AVPacket *pkt, int
> flush);
> @@ -889,17 +864,56 @@ 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;
> + AVPacket opkt = {0};
The doc of av_bsf_receive_packet() states that this packet "should be
"clean" (i.e. freshly allocated with av_packet_alloc() or unreffed with
av_packet_unref())". Simply zeroing it does not fulfill this requirement.
I suggest an alternative approach: We only need a spare packet for
av_bsf_receive_packet() if we don't already have a packet; and this
happens if and only if this function is called from av_write_trailer().
So this spare packet should reside in av_write_trailer() and be properly
initialized there (so that it can be used to signal EOF to the bsf); pkt
should be the only packet in this function. This avoids unnecessary
initializations.
> + 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, &opkt);
> + if (ret < 0) {
> + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
> + ret = 0;
You can just return 0 here and remove the next check.
> + if (ret < 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(&opkt, bsfc->time_base_out, st->time_base);
> + ret = write_packet_common(s, st, &opkt, interleaved);
> + if (!interleaved) // write_packet_common already unrefed opkt for
> interleaved
Unfortunately not. It does this only if interleaved_write_packet() has
been reached. If not, opkt leaks.
If you followed the above recommendation, then the memleak for the
interleaved codepath would be gone: The packet will be unreferenced in
av_interleaved_write_frame(); yet one still needs to unreference in the
av_write_trailer() codepath.
> + av_packet_unref(&opkt);
> + } 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 = need_auto_bsf(s, st, pkt);
> + if (ret < 0)
> return ret;
>
> - return write_packet_common(s, st, pkt, interleaved);
> + if (ret) {
> + 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)
> @@ -1254,9 +1268,18 @@ int av_interleaved_write_frame(AVFormatContext *s,
> AVPacket *pkt)
>
> int av_write_trailer(AVFormatContext *s)
> {
> - int ret, i;
> + int i, ret1, ret = 0;
>
> - ret = interleaved_write_packet(s, NULL, 1);
> + for (i = 0; i < s->nb_streams; i++) {
> + if (need_auto_bsf(s, s->streams[i], NULL)) {
It is better to simply check whether there this stream already has a bsf
or not. This allows you to remove the check for whether pkt is NULL or
not from need_auto_bsf() and it is generally nicer: After all, when
writing the trailer you do not want to check whether you still need to
add a bsf to a stream or not, because if this stream ever needed a bsf,
it should already have been added long ago.
> + ret1 = write_packets_from_bsfs(s, s->streams[i], NULL,
> 1/*interleaved*/);
> + 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-requ...@ffmpeg.org with subject "unsubscribe".
