[FFmpeg-devel] [PATCH 08/10] avcodec/flac_parser: Don't modify size of the input buffer

[Andreas Rheinhardt]

When flushing, MAX_FRAME_HEADER_SIZE bytes (always zero) are supposed to
be written to the fifo buffer in order to be able to check the rest of
the buffer for frame headers. It was intended to write these by writing
a small buffer of size MAX_FRAME_HEADER_SIZE to the buffer. But the way
it was actually done ensured that this did not happen:

First, it would be checked whether the size of the input buffer was zero,
in which case it buf_size would be set to MAX_FRAME_HEADER_SIZE and
read_end would be set to indicate that MAX_FRAME_HEADER_SIZE bytes need
to be written. Then it would be made sure that there is enough space in
the fifo for the data to be written. Afterwards the data is written. The
check used here is for whether buf_size is zero or not. But if it was
zero initially, it is MAX_FRAME_HEADER_SIZE now, so that not the
designated buffer for writing MAX_FRAME_HEADER_SIZE is written; instead
the padded buffer (from the stack of av_parser_parse2()) is used. This
works because AV_INPUT_BUFFER_PADDING_SIZE >= MAX_FRAME_HEADER_SIZE.
Lateron, buf_size is set to zero again.

Given that since 7edbd536, the actual amount of data read is no longer
automatically equal to buf_size, it is completely unnecessary to modify
buf_size at all. Moreover, modifying it is dangerous: Some allocations
can fail and because buf_size is never reset to zero in this codepath,
the parser might return a value > 0 on flushing.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
If I am not mistaken, then the code for writing the end padding was
always dead code (until now). (There even used to be a time when the
eight byte FF_INPUT_BUFFER_PADDING buffer was used to write 16 bytes
of padding.) 
Given that this has worked I am wondering whether it should not be done
intentionally. This would have the requirement of AV_INPUT_BUFFER_PADDING
being >= MAX_FRAME_HEADER_SIZE, but that should be certain given that
the former is usually only incremented. The current code btw. still
relies on this, because otherwise read_end = rest_start +
MAX_FRAME_HEADER_SIZE is undefined.
  
 libavcodec/flac_parser.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index 1dcb3ef2db..376ba2bcfc 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -595,7 +595,6 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
         /* Pad the end once if EOF, to check the final region for headers. */
         if (!buf_size) {
             fpc->end_padded      = 1;
-            buf_size = MAX_FRAME_HEADER_SIZE;
             read_end = read_start + MAX_FRAME_HEADER_SIZE;
         } else {
             /* The maximum read size is the upper-bound of what the parser
@@ -668,7 +667,6 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
                 fpc->fifo_buf->wptr += fpc->fifo_buf->end -
                     fpc->fifo_buf->buffer;
             }
-            buf_size = 0;
             read_start = read_end = NULL;
         }
     }
-- 
2.20.1