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 From patchwork Tue Apr 28 11:58:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lynne X-Patchwork-Id: 19314 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 D4F2944B34F for ; Tue, 28 Apr 2020 14:59:00 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BD9E968C09F; Tue, 28 Apr 2020 14:59:00 +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 E31BF68BC0A for ; Tue, 28 Apr 2020 14:58:53 +0300 (EEST) Received: from w3.tutanota.de (unknown [192.168.1.164]) by w4.tutanota.de (Postfix) with ESMTP id 91A2910602E3 for ; Tue, 28 Apr 2020 11:58:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1588075133; 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=ubsx8zhK4mxzmT68Je/krhAkVcdwicYm0HUGdig7dKA=; b=i8krXE+zCk7IkBPpgHL27wTZhTfvCjkRJJuitJaJM501254EGS5sjpJC2w/8fJX+ TfAthbr7qJdPK9qxQyya/gzgLmzvnzie7F23i/JKhV+xNCCxmrl0LCeCU+j3NAHfQ99 53YwqqvbEyVTAMccRq9njXkV/oiJfML75YWbjA9q6ZRDiIf26THjt4OX745afsVQ2ml SiQpkY91YXU2Vf4YjlMgKRwba8BwLnFyAZWbkR+jxW68E+4w8p0cDfBiHi9Krj58eDt ZUmtURm+QjjIfdqb7Klef6jWDCywWwdB7ERGBqn9o75haNwuJ6OI88in2KrE1HwZNna qBV9536uRg== Date: Tue, 28 Apr 2020 13:58:53 +0200 (CEST) From: Lynne To: Ffmpeg Devel Message-ID: MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/3] oggdec: verify page checksum 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 makes decoding far more robust, since OggS, the ogg magic, can be commonly found randomly in streams, which previously made the demuxer think there's a new stream or a change in such. Patch attached. --- libavformat/oggdec.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index f67cf42e82..05cea2528b 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -31,6 +31,7 @@ #include #include "libavutil/avassert.h" #include "libavutil/intreadwrite.h" +#include "avio_internal.h" #include "oggdec.h" #include "avformat.h" #include "internal.h" @@ -321,6 +322,7 @@ static int ogg_read_page(AVFormatContext *s, int *sid) int flags, nsegs; uint64_t gp; uint32_t serial; + uint32_t crc, crc_tmp; int size = 0, idx; int64_t page_pos; uint8_t sync[4]; @@ -359,6 +361,9 @@ static int ogg_read_page(AVFormatContext *s, int *sid) return AVERROR_INVALIDDATA; } + /* 0x4fa9b05f = av_crc(AV_CRC_32_IEEE, 0x0, "OggS", 4) */ + ffio_init_checksum(bc, ff_crc04C11DB7_update, 0x4fa9b05f); + if (avio_r8(bc) != 0) { /* version */ av_log (s, AV_LOG_ERROR, "ogg page, unsupported version\n"); return AVERROR_INVALIDDATA; @@ -367,7 +372,12 @@ static int ogg_read_page(AVFormatContext *s, int *sid) flags = avio_r8(bc); gp = avio_rl64(bc); serial = avio_rl32(bc); - avio_skip(bc, 8); /* seq, crc */ + avio_skip(bc, 4); /* seq */ + + crc_tmp = ffio_get_checksum(bc); + crc = avio_rb32(bc); + crc_tmp = ff_crc04C11DB7_update(crc_tmp, (uint8_t[4]){0}, 4); + ffio_init_checksum(bc, ff_crc04C11DB7_update, crc_tmp); nsegs = avio_r8(bc); page_pos = avio_tell(bc) - 27; @@ -407,6 +417,15 @@ static int ogg_read_page(AVFormatContext *s, int *sid) return ret < 0 ? ret : AVERROR_EOF; } + if (crc ^ ffio_get_checksum(bc)) { + av_log(s, AV_LOG_ERROR, "CRC mismatch!\n"); + if (idx < 0) + av_free(readout_buf); + avio_seek(bc, -size, SEEK_CUR); + return 0; + } + + /* CRC is correct so we can be 99% sure there's an actual change here */ if (idx < 0) { if (data_packets_seen(ogg)) idx = ogg_replace_stream(s, serial, size); -- 2.26.2 From patchwork Tue Apr 28 11:59:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lynne X-Patchwork-Id: 19315 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 BA05144B2FC for ; Tue, 28 Apr 2020 14:59:35 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A2D2468C21B; Tue, 28 Apr 2020 14:59:35 +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 C941D68C1BE for ; Tue, 28 Apr 2020 14:59:29 +0300 (EEST) Received: from w3.tutanota.de (unknown [192.168.1.164]) by w4.tutanota.de (Postfix) with ESMTP id 7857A10602D1 for ; Tue, 28 Apr 2020 11:59:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1588075169; 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=geYrGyvuq07nNKZn2DJSXevE+GsttSm6ARu42kBZ3wI=; b=qiITtyawz+ZnW28SkNhDgb+zWU8kGLv7svVfyj/SYPPADES5xeRc49UJf6GYPCXn NJ6p6TDWDGqf6pVVerVWikmruTqno2iK4RuvAIFIg+XZuX9A/3SdntjB5jKEyBIhM9g 0S+7/PcIxRafS51KQczvB2AnWpYW8+KyrK3LsflmXzgM9W4QY2ydbl73C7IOWW4x3mC QUu4UPDSb3JmN4Od4ZdVHsyi8YKGkx4KAUhEYyvLSbeBAoCrzNbv4jWaaZEIborwoX1 t73o6GhmugNiS3fXkr9nk961iqgx+fugYlAzvKnMkTwMnoLiU87ncrnh9XMUiKo7NYb /EvvMD5nlg== Date: Tue, 28 Apr 2020 13:59:29 +0200 (CEST) From: Lynne To: Ffmpeg Devel Message-ID: MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/3] oggdec: use ffio_ensure_seekback() to not require seeking to read the magic 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 just cleans up the code and simplifies it. Patch attached. --- libavformat/oggdec.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 05cea2528b..88d54dfe12 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -211,30 +211,30 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, int size) struct ogg *ogg = s->priv_data; struct ogg_stream *os; const struct ogg_codec *codec; + uint8_t magic[8]; int i = 0; - if (s->pb->seekable & AVIO_SEEKABLE_NORMAL) { - uint8_t magic[8]; - avio_seek(s->pb, -size, SEEK_CUR); - if (avio_read(s->pb, magic, sizeof(magic)) != sizeof(magic)) - return AVERROR_INVALIDDATA; - 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"); - return AVERROR_INVALIDDATA; - } - for (i = 0; i < ogg->nstreams; i++) { - if (ogg->streams[i].codec == codec) - break; - } - if (i >= ogg->nstreams) - return ogg_new_stream(s, serial); - } else if (ogg->nstreams != 1) { + if (ogg->nstreams != 1) { avpriv_report_missing_feature(s, "Changing stream parameters in multistream ogg"); return AVERROR_PATCHWELCOME; } + avio_seek(s->pb, -size, SEEK_CUR); + if (avio_read(s->pb, magic, sizeof(magic)) != sizeof(magic)) + return AVERROR_INVALIDDATA; + 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"); + return AVERROR_INVALIDDATA; + } + for (i = 0; i < ogg->nstreams; i++) { + if (ogg->streams[i].codec == codec) + break; + } + if (i >= ogg->nstreams) + return ogg_new_stream(s, serial); + os = &ogg->streams[i]; os->serial = serial; @@ -410,6 +410,9 @@ static int ogg_read_page(AVFormatContext *s, int *sid) readout_buf = av_malloc(size); } + /* To rewind if checksum is bad/check magic on switches */ + ffio_ensure_seekback(bc, size); + ret = avio_read(bc, readout_buf, size); if (ret < size) { if (idx < 0) -- 2.26.2