From patchwork Tue Apr 28 11:58:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lynne X-Patchwork-Id: 19313 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 06E1D44B34F for ; Tue, 28 Apr 2020 14:58:23 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E2BD468C173; Tue, 28 Apr 2020 14:58:22 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from w4.tutanota.de (w4.tutanota.de [81.3.6.165]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 577F368BA43 for ; Tue, 28 Apr 2020 14:58:16 +0300 (EEST) Received: from w3.tutanota.de (unknown [192.168.1.164]) by w4.tutanota.de (Postfix) with ESMTP id EACFF1060289 for ; Tue, 28 Apr 2020 11:58:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1588075095; s=s1; d=lynne.ee; h=From:From:To:To:Subject:Subject:Content-Description:Content-ID:Content-Type:Content-Type:Content-Transfer-Encoding:Cc:Date:Date:In-Reply-To:MIME-Version:MIME-Version:Message-ID:Message-ID:Reply-To:References:Sender; bh=NH8ONG+oa4ydKlJjSe21Dwx56pVpWTmp5p48gE1xkRM=; b=KL+JqrzZwcbDThGArplPRehHSEwNAbVqwUx35mTweiTZUsBkCt5L0NX/XT+xr56J g6Amgw3a4OZhSxtKLnRyrGrL5eeJ23WjXZ1phVabzN1lXdMljWOFrrfFNqA92XKUxie JoLgXjJnjDaUbkSsSzEKA5IbOTvfgXJ0Ma0ufG1RUAk0xBe4lhyv4iTpKg1o1YDyrOk rrHl6UD0v2AYj0cJ5aM28F4juQ+WiIrz15w13mcCrEHu1N2BR/qYTr/2joIw63To3Ya pJ0qBqKyQ3EMadkR9fGERxSGXXR+Bv5A7QGG5KOFTJ0bg3Z2VCWHHHMFjoAmbhxJ9WR zS6OqDQGWg== Date: Tue, 28 Apr 2020 13:58:15 +0200 (CEST) From: Lynne To: Ffmpeg Devel Message-ID: MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/3] oggdec: eliminate copies and extra buffers 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" This also makes implementing CRC checking far simpler and more robust. Patch attached. From 2a32f16108e6a424b20a15be4e06ebe6e92535a9 Mon Sep 17 00:00:00 2001 From: Lynne Date: Tue, 28 Apr 2020 12:41:34 +0100 Subject: [PATCH 1/3] oggdec: eliminate copies and extra buffers This also makes implementing CRC checking far simpler and more robust. --- libavformat/oggdec.c | 127 ++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 69 deletions(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 95190589ab..f67cf42e82 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -205,7 +205,7 @@ static const struct ogg_codec *ogg_find_codec(uint8_t *buf, int size) * situation where a new audio stream spawn (identified with a new serial) and * must replace the previous one (track switch). */ -static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, int nsegs) +static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, int size) { struct ogg *ogg = s->priv_data; struct ogg_stream *os; @@ -214,11 +214,10 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, int nsegs) if (s->pb->seekable & AVIO_SEEKABLE_NORMAL) { uint8_t magic[8]; - int64_t pos = avio_tell(s->pb); - avio_skip(s->pb, nsegs); + avio_seek(s->pb, -size, SEEK_CUR); if (avio_read(s->pb, magic, sizeof(magic)) != sizeof(magic)) return AVERROR_INVALIDDATA; - avio_seek(s->pb, pos, SEEK_SET); + avio_seek(s->pb, size - sizeof(magic), SEEK_CUR); codec = ogg_find_codec(magic, sizeof(magic)); if (!codec) { av_log(s, AV_LOG_ERROR, "Cannot identify new stream\n"); @@ -303,27 +302,6 @@ static int ogg_new_stream(AVFormatContext *s, uint32_t serial) return idx; } -static int ogg_new_buf(struct ogg *ogg, int idx) -{ - struct ogg_stream *os = ogg->streams + idx; - uint8_t *nb = av_malloc(os->bufsize + AV_INPUT_BUFFER_PADDING_SIZE); - int size = os->bufpos - os->pstart; - - if (!nb) - return AVERROR(ENOMEM); - - if (os->buf) { - memcpy(nb, os->buf + os->pstart, size); - av_free(os->buf); - } - - os->buf = nb; - os->bufpos = size; - os->pstart = 0; - - return 0; -} - static int data_packets_seen(const struct ogg *ogg) { int i; @@ -343,8 +321,11 @@ static int ogg_read_page(AVFormatContext *s, int *sid) int flags, nsegs; uint64_t gp; uint32_t serial; - int size, idx; + int size = 0, idx; + int64_t page_pos; uint8_t sync[4]; + uint8_t segments[255]; + uint8_t *readout_buf; int sp = 0; ret = avio_read(bc, sync, 4); @@ -387,47 +368,73 @@ static int ogg_read_page(AVFormatContext *s, int *sid) gp = avio_rl64(bc); serial = avio_rl32(bc); avio_skip(bc, 8); /* seq, crc */ - nsegs = avio_r8(bc); + + nsegs = avio_r8(bc); + page_pos = avio_tell(bc) - 27; + + ret = avio_read(bc, segments, nsegs); + if (ret < nsegs) + return ret < 0 ? ret : AVERROR_EOF; if (avio_feof(bc)) return AVERROR_EOF; + for (i = 0; i < nsegs; i++) + size += segments[i]; + idx = ogg_find_stream(ogg, serial); + if (idx >= 0) { + os = ogg->streams + idx; + + /* Even if invalid guarantee there's enough memory to read the page */ + if (os->bufsize - os->bufpos < size) { + uint8_t *nb = av_realloc(os->buf, 2*os->bufsize + AV_INPUT_BUFFER_PADDING_SIZE); + if (!nb) + return AVERROR(ENOMEM); + os->buf = nb; + os->bufsize *= 2; + } + + readout_buf = os->buf + os->bufpos; + } else { + readout_buf = av_malloc(size); + } + + ret = avio_read(bc, readout_buf, size); + if (ret < size) { + if (idx < 0) + av_free(readout_buf); + return ret < 0 ? ret : AVERROR_EOF; + } + if (idx < 0) { if (data_packets_seen(ogg)) - idx = ogg_replace_stream(s, serial, nsegs); + idx = ogg_replace_stream(s, serial, size); else idx = ogg_new_stream(s, serial); if (idx < 0) { av_log(s, AV_LOG_ERROR, "failed to create or replace stream\n"); + av_free(readout_buf); return idx; } - } - os = ogg->streams + idx; - ogg->page_pos = - os->page_pos = avio_tell(bc) - 27; + os = ogg->streams + idx; - if (os->psize > 0) { - ret = ogg_new_buf(ogg, idx); - if (ret < 0) - return ret; + memcpy(os->buf + os->bufpos, readout_buf, size); + av_free(readout_buf); } - ret = avio_read(bc, os->segments, nsegs); - if (ret < nsegs) - return ret < 0 ? ret : AVERROR_EOF; - - os->nsegs = nsegs; - os->segp = 0; - - size = 0; - for (i = 0; i < nsegs; i++) - size += os->segments[i]; - - if (!(flags & OGG_FLAG_BOS)) - os->got_data = 1; + ogg->page_pos = page_pos; + os->page_pos = page_pos; + os->nsegs = nsegs; + os->segp = 0; + os->got_data = !(flags & OGG_FLAG_BOS); + os->bufpos += size; + os->granule = gp; + os->flags = flags; + memcpy(os->segments, segments, nsegs); + memset(os->buf + os->bufpos, 0, AV_INPUT_BUFFER_PADDING_SIZE); if (flags & OGG_FLAG_CONT || os->incomplete) { if (!os->psize) { @@ -447,26 +454,8 @@ static int ogg_read_page(AVFormatContext *s, int *sid) os->sync_pos = os->page_pos; } - if (os->bufsize - os->bufpos < size) { - uint8_t *nb = av_malloc((os->bufsize *= 2) + AV_INPUT_BUFFER_PADDING_SIZE); - if (!nb) - return AVERROR(ENOMEM); - memcpy(nb, os->buf, os->bufpos); - av_free(os->buf); - os->buf = nb; - } - - ret = avio_read(bc, os->buf + os->bufpos, size); - if (ret < size) - return ret < 0 ? ret : AVERROR_EOF; - - os->bufpos += size; - os->granule = gp; - os->flags = flags; - - memset(os->buf + os->bufpos, 0, AV_INPUT_BUFFER_PADDING_SIZE); - if (sid) - *sid = idx; + /* This function is always called with sid != NULL */ + *sid = idx; return 0; } -- 2.26.2