From patchwork Sun Oct 6 05:01:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 15525 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id ABE38449629 for ; Sun, 6 Oct 2019 08:16:46 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8A24E680C8A; Sun, 6 Oct 2019 08:16:46 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BC7AC6807F6 for ; Sun, 6 Oct 2019 08:16:40 +0300 (EEST) Received: by mail-wm1-f67.google.com with SMTP id b24so9244359wmj.5 for ; Sat, 05 Oct 2019 22:16:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=hU8ztOgwUxTymCTud5re/ocR0eMCroiAXK7K5i6NggY=; b=BJYeypOKLoODSHDuDrkDlGXZ4CYmuI83lAa6DyadLP5SzbBl2AE9yeQU1I5tEsQ2Q9 JTBGYGXB9+aIrN5Ynx1EWdM2cuC51yu/kW3d4osDJJdiaIUI8p9q+h8qGXrG/6TTanll yCyuOMK6efcywAkbY4hce5dw81LjvUdXaDnB/kHf/69SeNJQXIliMuxjRGxK2mXn52sG 3OWqcQDnq1/9W6W6aHiWIGd5CSXGQEoXtIrwWvY9qmzuN1g6OctATnAl5jVMwIaeeVpK lWsgbDGdEr4IsKVcosDMmmHb3ZAv6F/7e0kzuJlbEIpITYLDJNGkxvP8pn9brTdA1eLV 9vcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=hU8ztOgwUxTymCTud5re/ocR0eMCroiAXK7K5i6NggY=; b=iUPFAsyivZz85FmogSM9If3KV02K1cXL+VL1QHnwWvDS5xXgzQcnvIKZgufw2PHEiK 9s5eufl333vl+BAxBvcCZDt9yUiNAkjMIe3GtFl9fVUMJNxoikzbjI8GjP95aKYithBY WdrzLpVDgMPZILCzWpyaKKpcxQMHkPk9A5ND+kncZZXnt2Jr5GD+dm8D21dA0jV/AMi8 2h58Fe0XRKdpdJbige2V6kYpoLfpCC3pCt6O1x5Y71wA5M+CRshG4sxOiWzishyjcAUf 3qxj0vxT8e+ADAQWZkhi1yPdkT4qs3gEAQ+JzNoNid+3sJS0LdEVLk8crwVDLqbnfn7x hOPQ== X-Gm-Message-State: APjAAAVHZ2P00nUKx16YSx1LMSPnDxDqDj9zxSAPp4s+G9EzBx4zQ0LV TEUfYU3ZauhBNNRPmtwaLRrI6MTRWQE= X-Google-Smtp-Source: APXvYqwo5Mlb7sTWusAt20lwtbqCzofEV7sXp2YfrdY6TD/U5gMJfjJEUfCR05CGVhtnPBlViD2lAA== X-Received: by 2002:a1c:2d12:: with SMTP id t18mr7069443wmt.51.1570338630167; Sat, 05 Oct 2019 22:10:30 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08937.dynamic.kabel-deutschland.de. [188.192.137.55]) by smtp.gmail.com with ESMTPSA id s10sm18543014wmf.48.2019.10.05.22.10.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 05 Oct 2019 22:10:29 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 6 Oct 2019 07:01:18 +0200 Message-Id: <20191006050120.26807-8-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191006050120.26807-1-andreas.rheinhardt@gmail.com> References: <20191006050120.26807-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 08/10] avcodec/flac_parser: Don't modify size of the input buffer X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- 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; } }