| Message ID | 20220322163911.64772-2-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v3,1/2] lavf/mpegenc: fix ever-growing fifo size since the new API | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Nicolas Gaullier: > Avoid an infinite 'retry' loop in output_packet when flushing. > > A fatal error mentions the availability of fifo_size_limit option. > > Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> > --- > libavformat/mpegenc.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c > index 5d755e3bdd..eff4531037 100644 > --- a/libavformat/mpegenc.c > +++ b/libavformat/mpegenc.c > @@ -85,6 +85,7 @@ typedef struct MpegMuxContext { > > int preload; > uint32_t fifo_size_limit; > + int fifo_size_exceeded; > } MpegMuxContext; > > extern const AVOutputFormat ff_mpeg1vcd_muxer; > @@ -1153,7 +1154,7 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) > StreamInfo *stream = st->priv_data; > int64_t pts, dts; > PacketDesc *pkt_desc; > - int preload; > + int preload, ret; > const int is_iframe = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && > (pkt->flags & AV_PKT_FLAG_KEY); > > @@ -1220,10 +1221,17 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) > } > } > > - av_fifo_write(stream->fifo, buf, size); > + ret = av_fifo_write(stream->fifo, buf, size); > + if (ret == AVERROR(ENOSPC)) { > + s->fifo_size_exceeded = 1; > + av_log(s, AV_LOG_FATAL, "Input stream buffer overrun. " > + "To avoid, increase fifo_size_limit option.\n"); > + } > + if (ret < 0) > + return ret; > > for (;;) { > - int ret = output_packet(ctx, 0); > + ret = output_packet(ctx, 0); > if (ret <= 0) > return ret; > } > @@ -1231,9 +1239,13 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) > > static int mpeg_mux_end(AVFormatContext *ctx) > { > + MpegMuxContext *s = ctx->priv_data; > StreamInfo *stream; > int i; > > + if (s->fifo_size_exceeded) > + return 0; > + > for (;;) { > int ret = output_packet(ctx, 1); > if (ret < 0) Could this infinite loop also happen before switching to the new API? Does it happen because avail_data is zero for all streams? - Andreas
>Could this infinite loop also happen before switching to the new API? >Does it happen because avail_data is zero for all streams? > >- Andreas I will take time to double-check this very carefully, but in my experience, if I remember correctly, it is nothing easy and was already buggy before. (but with the older API, it was almost impossible to reach such conditions). Nicolas
>>Could this infinite loop also happen before switching to the new API? >>Does it happen because avail_data is zero for all streams? >> >>- Andreas > >I will take time to double-check this very carefully, but in my experience, if I remember correctly, it is nothing easy and was already buggy before. >(but with the older API, it was almost impossible to reach such conditions). Well, I really don't know how I missed that but indeed, no avail_data... So, I am doing some testing and post right afterwards an updated patch with an 'has_avail_data' check. The problem happen before the new API, for a long time ago (I checked about one year ago). Thanks to you! Nicolas
diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c index 5d755e3bdd..eff4531037 100644 --- a/libavformat/mpegenc.c +++ b/libavformat/mpegenc.c @@ -85,6 +85,7 @@ typedef struct MpegMuxContext { int preload; uint32_t fifo_size_limit; + int fifo_size_exceeded; } MpegMuxContext; extern const AVOutputFormat ff_mpeg1vcd_muxer; @@ -1153,7 +1154,7 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) StreamInfo *stream = st->priv_data; int64_t pts, dts; PacketDesc *pkt_desc; - int preload; + int preload, ret; const int is_iframe = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && (pkt->flags & AV_PKT_FLAG_KEY); @@ -1220,10 +1221,17 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) } } - av_fifo_write(stream->fifo, buf, size); + ret = av_fifo_write(stream->fifo, buf, size); + if (ret == AVERROR(ENOSPC)) { + s->fifo_size_exceeded = 1; + av_log(s, AV_LOG_FATAL, "Input stream buffer overrun. " + "To avoid, increase fifo_size_limit option.\n"); + } + if (ret < 0) + return ret; for (;;) { - int ret = output_packet(ctx, 0); + ret = output_packet(ctx, 0); if (ret <= 0) return ret; } @@ -1231,9 +1239,13 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) static int mpeg_mux_end(AVFormatContext *ctx) { + MpegMuxContext *s = ctx->priv_data; StreamInfo *stream; int i; + if (s->fifo_size_exceeded) + return 0; + for (;;) { int ret = output_packet(ctx, 1); if (ret < 0)
Avoid an infinite 'retry' loop in output_packet when flushing. A fatal error mentions the availability of fifo_size_limit option. Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> --- libavformat/mpegenc.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)