| Message ID | 20220318131504.18488-1-nicolas.gaullier@cji.paris |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] lavf/mpegenc: fix memory leak in the fifo since the new API | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
Quoting Nicolas Gaullier (2022-03-18 14:15:04) > The older av_fifo_realloc2 implemented an auto grow that should be ported as such. > > This introduces a limitation in the buffer size. > AUTO_GROW_DEFAULT_BYTES is currently 1MB which seems reasonable here. Are you sure it's enough? 1MB packets are not unthinkable for high-bitrate video.
Nicolas Gaullier: > The older av_fifo_realloc2 implemented an auto grow that should be ported as such. > > This introduces a limitation in the buffer size. > AUTO_GROW_DEFAULT_BYTES is currently 1MB which seems reasonable here. > > Fix regressing since ea511196a6c85eb433e10cdbecb0b2c722faf20d > > Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> > --- > libavformat/mpegenc.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c > index cc47a43288..ba0b5d813a 100644 > --- a/libavformat/mpegenc.c > +++ b/libavformat/mpegenc.c > @@ -461,7 +461,7 @@ 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); > } > @@ -1151,7 +1151,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 +1207,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 && You mean an ever-growing buffer when talking about a "memory leak"? Because I actually don't see any ordinary memory leak. - Andreas
>You mean an ever-growing buffer when talking about a "memory leak"? >Because I actually don't see any ordinary memory leak. >- Andreas Yes, that's it, the fifo grows at every write. I will fix the commit msg in the next version. Nicolas
diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c index cc47a43288..ba0b5d813a 100644 --- a/libavformat/mpegenc.c +++ b/libavformat/mpegenc.c @@ -461,7 +461,7 @@ 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); } @@ -1151,7 +1151,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 +1207,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 &&
The older av_fifo_realloc2 implemented an auto grow that should be ported as such. This introduces a limitation in the buffer size. AUTO_GROW_DEFAULT_BYTES is currently 1MB which seems reasonable here. Fix regressing since ea511196a6c85eb433e10cdbecb0b2c722faf20d Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris> --- libavformat/mpegenc.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)