Re: [FFmpeg-devel] [PATCH 5/8] avformat/mux: add proper support for full N:M bitstream filtering
On Sat, 28 Mar 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.
Signed-off-by: Marton Balint
---
libavformat/internal.h | 1 +
libavformat/mux.c | 128 ++---
2 files changed, 91 insertions(+), 38 deletions(-)
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 332477a532..45aeef717a 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -158,6 +158,7 @@ struct AVStreamInternal {
*/
AVBSFContext **bsfcs;
int nb_bsfcs;
+int bsfcs_idx;
/**
* Whether or not check_bitstream should still be run on each packet
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 8c2d6a8060..3054ab8644 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -840,14 +840,48 @@ 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];
-int i, ret;
+static int auto_bsf_receive_packet(AVFormatContext *s, AVStream *st, AVPacket
*pkt)
+{
+AVStreamInternal *sti = st->internal;
+int ret = AVERROR(EAGAIN);
+int eof = 0;
+
+while (sti->bsfcs_idx) {
+/* get a packet from the previous filter up the chain */
+ret = av_bsf_receive_packet(sti->bsfcs[sti->bsfcs_idx - 1], pkt);
+if (ret == AVERROR(EAGAIN)) {
+sti->bsfcs_idx--;
+continue;
+} else if (ret == AVERROR_EOF) {
+eof = 1;
+} else if (ret < 0)
+break;
+
+/* send it to the next filter down the chain */
+if (sti->bsfcs_idx < sti->nb_bsfcs) {
+ret = av_bsf_send_packet(sti->bsfcs[sti->bsfcs_idx], eof ? NULL :
pkt);
+av_assert2(ret != AVERROR(EAGAIN));
+if (ret < 0)
+break;
+sti->bsfcs_idx++;
+eof = 0;
+} else if (eof) {
+break;
+} else {
+return 0;
+}
+}
Would it actually be possible to simplify this by using the
av_bsf_list-API (right now code like this exists here and in ffmpeg.c
and probably at even more places)? The problem I see with this is that
one cannot send a packet for filtering to an unfinished bsf list whereas
the current code has this capability (although AFAIK no one uses it; if
I am not mistaken, all bsf chains that are automatically inserted
consist of exactly one bsf).
I am not aware of practical uses of more than one bsf, but it *was*
supported for a long time.
Yes, the main issue with the list_bsf-API is that it does not currently
supports adding filters to already initialized chains. I wrapped up some
new API for it which can be used to do exactly that:
+/**
+ * Join a new bitstream filter to an already operating one.
+ *
+ * @param[in,out] out_bsf *out_bsf contains the existing, already initialized
bitstream
+ *filter, the new filter will be joined to the output
of this.
+ *It can be NULL, in which case an empty filter is
assumed.
+ *Upon successful return *out_bsf will contain the
new, combined
+ *bitstream filter which will be initialized.
+ * @param[in] bsf the bitstream filter to be joined to the output of
*out_bsf.
+ *This filter must be uninitialized but it must be
ready to be
+ *initalized, so input codec parameters and time base
must be
+ *set if *out_bsf is NULL, otherwise the function will
set these
+ *automatically based on the output parameters of
*out_bsf.
+ *Upon successful return the bsf will be initialized.
+ *
+ * @return 0 on success, negative on error.
+ */
+int av_bsf_join(AVBSFContext **out_bsf, AVBSFContext *bsf);
+
So this API basically creates a list BSF transparenly if somebody wants to
concatenate two BSF-s. It is a bit fraglie because it is using list bfs
internals directly, instead of calling the init() method of the list bsf,
but it works.
If people prefer this option, I can continue on this path and simply
provide patches on top of the existing series, it is quite
straightforward to do the conversion.
(This actually brings up a question: If check_bitstream returns that
more packets from this stream need to be checked, the current packet is
currently sent to the bsf-list as-is; it would not be sent to any bsf
that get added to the list later. Makes me wonder whether this is
actually a problem.)
I always thought that is how it is supposed to work by design. But since
there are no use cases for that at the moment, we don't know if that is
satisfactory...
Regards,
Marton
___
ff
Re: [FFmpeg-devel] [PATCH 5/8] avformat/mux: add proper support for full N:M bitstream filtering
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
> ---
> libavformat/internal.h | 1 +
> libavformat/mux.c | 128
> ++---
> 2 files changed, 91 insertions(+), 38 deletions(-)
>
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 332477a532..45aeef717a 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -158,6 +158,7 @@ struct AVStreamInternal {
> */
> AVBSFContext **bsfcs;
> int nb_bsfcs;
> +int bsfcs_idx;
>
> /**
> * Whether or not check_bitstream should still be run on each packet
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 8c2d6a8060..3054ab8644 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -840,14 +840,48 @@ 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];
> -int i, ret;
> +static int auto_bsf_receive_packet(AVFormatContext *s, AVStream *st,
> AVPacket *pkt)
> +{
> +AVStreamInternal *sti = st->internal;
> +int ret = AVERROR(EAGAIN);
> +int eof = 0;
> +
> +while (sti->bsfcs_idx) {
> +/* get a packet from the previous filter up the chain */
> +ret = av_bsf_receive_packet(sti->bsfcs[sti->bsfcs_idx - 1], pkt);
> +if (ret == AVERROR(EAGAIN)) {
> +sti->bsfcs_idx--;
> +continue;
> +} else if (ret == AVERROR_EOF) {
> +eof = 1;
> +} else if (ret < 0)
> +break;
> +
> +/* send it to the next filter down the chain */
> +if (sti->bsfcs_idx < sti->nb_bsfcs) {
> +ret = av_bsf_send_packet(sti->bsfcs[sti->bsfcs_idx], eof ? NULL
> : pkt);
> +av_assert2(ret != AVERROR(EAGAIN));
> +if (ret < 0)
> +break;
> +sti->bsfcs_idx++;
> +eof = 0;
> +} else if (eof) {
> +break;
> +} else {
> +return 0;
> +}
> +}
> +
> +return ret;
> +}
> +
> +static int need_auto_bsf(AVFormatContext *s, AVStream *st, AVPacket *pkt) {
{ belongs into 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;
> @@ -856,31 +890,7 @@ 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];
> -// 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->nb_bsfcs;
> }
>
> static int interleaved_write_packet(AVFormatContext *s, AVPacket *pkt, int
> flush);
> @@ -909,19 +919,54 @@ static int write_packet_common(AVFormatContext *s,
> AVStream *st, AVPacket *pkt,
>
> static int write_packets_common(AVFormatContext *s, AVStream *st, AVPacket
> *pkt, int interleaved)
> {
> -int ret = do_packet_auto_bsf(s, pkt);
> -if (ret == 0)
> -return 0;
> -else if (ret < 0)
> +AVStreamInternal *sti = st->internal;
> +int ret;
> +
> +ret = need_auto_bsf(s, st, pkt);
> +if (ret < 0)
> goto fail;
>
> -ret = write_packet_common(s, st, pkt, interleaved);
> +if (ret) {
> +AVPacket opkt = {0};
> +int consumed_packet = 0;
> +
> +do {
> +ret = auto_bsf_receive_packet(s, st, &opkt);
> +
Re: [FFmpeg-devel] [PATCH 5/8] avformat/mux: add proper support for full N:M bitstream filtering
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
> ---
> libavformat/internal.h | 1 +
> libavformat/mux.c | 128
> ++---
> 2 files changed, 91 insertions(+), 38 deletions(-)
>
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 332477a532..45aeef717a 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -158,6 +158,7 @@ struct AVStreamInternal {
> */
> AVBSFContext **bsfcs;
> int nb_bsfcs;
> +int bsfcs_idx;
>
> /**
> * Whether or not check_bitstream should still be run on each packet
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 8c2d6a8060..3054ab8644 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -840,14 +840,48 @@ 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];
> -int i, ret;
> +static int auto_bsf_receive_packet(AVFormatContext *s, AVStream *st,
> AVPacket *pkt)
> +{
> +AVStreamInternal *sti = st->internal;
> +int ret = AVERROR(EAGAIN);
> +int eof = 0;
> +
> +while (sti->bsfcs_idx) {
> +/* get a packet from the previous filter up the chain */
> +ret = av_bsf_receive_packet(sti->bsfcs[sti->bsfcs_idx - 1], pkt);
> +if (ret == AVERROR(EAGAIN)) {
> +sti->bsfcs_idx--;
> +continue;
> +} else if (ret == AVERROR_EOF) {
> +eof = 1;
> +} else if (ret < 0)
> +break;
> +
> +/* send it to the next filter down the chain */
> +if (sti->bsfcs_idx < sti->nb_bsfcs) {
> +ret = av_bsf_send_packet(sti->bsfcs[sti->bsfcs_idx], eof ? NULL
> : pkt);
> +av_assert2(ret != AVERROR(EAGAIN));
> +if (ret < 0)
> +break;
> +sti->bsfcs_idx++;
> +eof = 0;
> +} else if (eof) {
> +break;
> +} else {
> +return 0;
> +}
> +}
Would it actually be possible to simplify this by using the
av_bsf_list-API (right now code like this exists here and in ffmpeg.c
and probably at even more places)? The problem I see with this is that
one cannot send a packet for filtering to an unfinished bsf list whereas
the current code has this capability (although AFAIK no one uses it; if
I am not mistaken, all bsf chains that are automatically inserted
consist of exactly one bsf).
(This actually brings up a question: If check_bitstream returns that
more packets from this stream need to be checked, the current packet is
currently sent to the bsf-list as-is; it would not be sent to any bsf
that get added to the list later. Makes me wonder whether this is
actually a problem.)
- Andreas
___
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".
[FFmpeg-devel] [PATCH 5/8] avformat/mux: add proper support for full N:M bitstream filtering
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
---
libavformat/internal.h | 1 +
libavformat/mux.c | 128 ++---
2 files changed, 91 insertions(+), 38 deletions(-)
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 332477a532..45aeef717a 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -158,6 +158,7 @@ struct AVStreamInternal {
*/
AVBSFContext **bsfcs;
int nb_bsfcs;
+int bsfcs_idx;
/**
* Whether or not check_bitstream should still be run on each packet
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 8c2d6a8060..3054ab8644 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -840,14 +840,48 @@ 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];
-int i, ret;
+static int auto_bsf_receive_packet(AVFormatContext *s, AVStream *st, AVPacket
*pkt)
+{
+AVStreamInternal *sti = st->internal;
+int ret = AVERROR(EAGAIN);
+int eof = 0;
+
+while (sti->bsfcs_idx) {
+/* get a packet from the previous filter up the chain */
+ret = av_bsf_receive_packet(sti->bsfcs[sti->bsfcs_idx - 1], pkt);
+if (ret == AVERROR(EAGAIN)) {
+sti->bsfcs_idx--;
+continue;
+} else if (ret == AVERROR_EOF) {
+eof = 1;
+} else if (ret < 0)
+break;
+
+/* send it to the next filter down the chain */
+if (sti->bsfcs_idx < sti->nb_bsfcs) {
+ret = av_bsf_send_packet(sti->bsfcs[sti->bsfcs_idx], eof ? NULL :
pkt);
+av_assert2(ret != AVERROR(EAGAIN));
+if (ret < 0)
+break;
+sti->bsfcs_idx++;
+eof = 0;
+} else if (eof) {
+break;
+} else {
+return 0;
+}
+}
+
+return ret;
+}
+
+static int need_auto_bsf(AVFormatContext *s, AVStream *st, AVPacket *pkt) {
+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;
@@ -856,31 +890,7 @@ 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];
-// 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->nb_bsfcs;
}
static int interleaved_write_packet(AVFormatContext *s, AVPacket *pkt, int
flush);
@@ -909,19 +919,54 @@ static int write_packet_common(AVFormatContext *s,
AVStream *st, AVPacket *pkt,
static int write_packets_common(AVFormatContext *s, AVStream *st, AVPacket
*pkt, int interleaved)
{
-int ret = do_packet_auto_bsf(s, pkt);
-if (ret == 0)
-return 0;
-else if (ret < 0)
+AVStreamInternal *sti = st->internal;
+int ret;
+
+ret = need_auto_bsf(s, st, pkt);
+if (ret < 0)
goto fail;
-ret = write_packet_common(s, st, pkt, interleaved);
+if (ret) {
+AVPacket opkt = {0};
+int consumed_packet = 0;
+
+do {
+ret = auto_bsf_receive_packet(s, st, &opkt);
+if (ret < 0) {
+if (ret == AVERROR(EAGAIN) && !consumed_packet) {
+av_assert2(sti->bsfcs_idx == 0);
+ret = av_bsf_send_packet(sti->bsfcs[0], pkt);
+av_assert2(ret != AVERROR(EAGAIN));
+if (ret >= 0) {
+consumed_packet = 1;
+
