[FFmpeg-devel] [PATCH] avformat/mpegenc: Fix ever growing FIFO and infinite loop on error

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Since the switch to the new FIFO API in commit
> ea511196a6c85eb433e10cdbecb0b2c722faf20d, the FIFO is always
> grown by the amount of data intended to be written into it
> even in case the FIFO has enough free space. Fix this by
> only growing the FIFO if needed and then only by the amount that is
> actually needed.
> 
> The allocation errors that resulted from this uncovered another bug:
> The context is left in an inconsistent state in case the FIFO can't
> be grown, because the FIFO does not contain as much data as the sizes
> contained in the PacketDesc list claim. This led to an infinite loop
> in output_packet() (called from mpeg_mux_end()).
> 
> Fix this by growing the FIFO before adding a new PacketDesc element,
> thereby preventing the context from becoming inconsistent.
> 
> Reported-by: Nicolas Gaullier <nicolas.gaullier at cji.paris>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  libavformat/mpegenc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
> index cc47a43288..62692bfcd1 100644
> --- a/libavformat/mpegenc.c
> +++ b/libavformat/mpegenc.c
> @@ -1152,6 +1152,7 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt)
>      int64_t pts, dts;
>      PacketDesc *pkt_desc;
>      int preload, ret;
> +    size_t can_write;
>      const int is_iframe = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
>                            (pkt->flags & AV_PKT_FLAG_KEY);
>  
> @@ -1192,6 +1193,14 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt)
>          size -= 3;
>      }
>  
> +    /* Enlarge the FIFO before adding a new PacketDesc
> +     * in order to avoid inconsistencies on failure. */
> +    can_write = av_fifo_can_write(stream->fifo);
> +    if (can_write < size) {
> +        ret = av_fifo_grow2(stream->fifo, size - can_write);
> +        if (ret < 0)
> +            return ret;
> +    }
>      pkt_desc                 = av_mallocz(sizeof(PacketDesc));
>      if (!pkt_desc)
>          return AVERROR(ENOMEM);
> @@ -1207,10 +1216,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 &&

Will apply this tonight unless there are objections.

- Andreas