Nicolas Gaullier:
> The older av_fifo_realloc2 implemented an auto grow that
> should be ported as such in the new API.
>
> This patch introduces a limitation in the fifo buffer size.
> The default is set to 128MB and may be overriden by a new user option.
> The amount of memory allocated depends on multiple factors, including
> the number of audio streams.
> A worst case scenario is where an out-of-spec high video bitrate is
> combined with numerous low bitrate audios.
>
> Fix regressing since ea511196a6c85eb433e10cdbecb0b2c722faf20d
>
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
> libavformat/mpegenc.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
> index cc47a43288..5d755e3bdd 100644
> --- a/libavformat/mpegenc.c
> +++ b/libavformat/mpegenc.c
> @@ -84,6 +84,7 @@ typedef struct MpegMuxContext {
> int64_t vcd_padding_bytes_written;
>
> int preload;
> + uint32_t fifo_size_limit;
> } MpegMuxContext;
>
> extern const AVOutputFormat ff_mpeg1vcd_muxer;
> @@ -461,9 +462,10 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
> av_get_media_type_string(st->codecpar->codec_type), i);
> return AVERROR(EINVAL);
> }
> - stream->fifo = av_fifo_alloc2(16, 1, 0);
> + stream->fifo = av_fifo_alloc2(16, 1, AV_FIFO_FLAG_AUTO_GROW);
> if (!stream->fifo)
> return AVERROR(ENOMEM);
> + av_fifo_auto_grow_limit(stream->fifo, s->fifo_size_limit);
> }
> bitrate = 0;
> audio_bitrate = 0;
> @@ -1151,7 +1153,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, ret;
> + int preload;
> const int is_iframe = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> (pkt->flags & AV_PKT_FLAG_KEY);
>
> @@ -1207,10 +1209,6 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx,
> AVPacket *pkt)
> pkt_desc->unwritten_size =
> pkt_desc->size = size;
>
> - ret = av_fifo_grow2(stream->fifo, size);
> - if (ret < 0)
> - return ret;
> -
> if (s->is_dvd) {
> // min VOBU length 0.4 seconds (mpucoder)
> if (is_iframe &&
> @@ -1277,6 +1275,7 @@ static void mpeg_mux_deinit(AVFormatContext *ctx)
> static const AVOption options[] = {
> { "muxrate", NULL,
> OFFSET(user_mux_rate), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, ((1<<22) - 1) * (8 *
> 50), E },
> { "preload", "Initial demux-decode delay in microseconds.",
> OFFSET(preload), AV_OPT_TYPE_INT, { .i64 = 500000 }, 0, INT_MAX, E },
> + { "fifo_size_limit", "Maximum allowed memory for buffering an input
> stream in bytes", OFFSET(fifo_size_limit), AV_OPT_TYPE_INT, {.i64 = 128 *
> 1024 * 1024 }, 0, UINT_MAX, E},
> { NULL },
> };
>
1. Options of type AV_OPT_TYPE_INT need to have a target of type int.
2. Setting UINT_MAX as maximum for such an option is nonsense; INT_MAX
is the maximum for it. (FFMIN(INT_MAX, SIZE_MAX) would be even better.)
3. Allowing zero for fifo_size_limit makes no sense, as the above code
allocates 16 when allocating the FIFO.
4. After removing av_fifo_grow2() it is no longer assured that
av_fifo_write() succeeds; it therefore needs to be checked.
- 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".
