Apr 28, 2020, 14:05 by mattias.wad...@gmail.com:
> On Tue, Apr 28, 2020 at 1:59 PM Lynne <d...@lynne.ee> wrote:
>
>>
>> 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.
>>
>
> Maybe nitpick, could seek back even longer than size on crc mismatch?
>
Thanks a lot for reviewing the patches.
I was thinking about seeking back further to perhaps the version byte, but
unfortunately, that's
not possible. If ffio_ensure_seekback is called multiple times, the last call
overwrites the previous
calls and we lose the ability to seek before it.
Since the only place at which we know the exact size of the page is when its
signaled, that's the
only point we can ensure we can seek back to.
Before that, we can seek back from the header's end to the version byte.
Unfortunately, that
would mean verifying the header before the checksum, which as you've pointed
out, is bad
for robustness.
Still, this is definitely an improvement, since previously the demuxer didn't
seek back at all.
Anyway, I've implemented checking the version byte after reading the CRC as you
suggested
and have attached the new patch.
>From 989d02631f2b47f125d3d8a3474572fe25ab1251 Mon Sep 17 00:00:00 2001
From: Lynne <d...@lynne.ee>
Date: Tue, 28 Apr 2020 12:52:11 +0100
Subject: [PATCH 2/3] oggdec: verify page checksum
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.
---
libavformat/oggdec.c | 46 ++++++++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
index 7db26840b2..e0188c7c59 100644
--- a/libavformat/oggdec.c
+++ b/libavformat/oggdec.c
@@ -31,6 +31,7 @@
#include <stdio.h>
#include "libavutil/avassert.h"
#include "libavutil/intreadwrite.h"
+#include "avio_internal.h"
#include "oggdec.h"
#include "avformat.h"
#include "internal.h"
@@ -321,8 +322,9 @@ 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;
+ int64_t version, page_pos;
uint8_t sync[4];
uint8_t segments[255];
uint8_t *readout_buf;
@@ -359,15 +361,19 @@ static int ogg_read_page(AVFormatContext *s, int *sid)
return AVERROR_INVALIDDATA;
}
- if (avio_r8(bc) != 0) { /* version */
- av_log (s, AV_LOG_ERROR, "ogg page, unsupported version\n");
- return AVERROR_INVALIDDATA;
- }
+ /* 0x4fa9b05f = av_crc(AV_CRC_32_IEEE, 0x0, "OggS", 4) */
+ ffio_init_checksum(bc, ff_crc04C11DB7_update, 0x4fa9b05f);
- flags = avio_r8(bc);
- gp = avio_rl64(bc);
- serial = avio_rl32(bc);
- avio_skip(bc, 8); /* seq, crc */
+ version = avio_r8(bc);
+ flags = avio_r8(bc);
+ gp = avio_rl64(bc);
+ serial = avio_rl32(bc);
+ 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;
@@ -376,9 +382,6 @@ static int ogg_read_page(AVFormatContext *s, int *sid)
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];
@@ -407,6 +410,25 @@ 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;
+ }
+
+ /* Since we're almost sure its a valid packet, checking the version after
+ * the checksum lets the demuxer be more tolerant */
+ if (version) {
+ av_log(s, AV_LOG_ERROR, "Invalid Ogg vers!\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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
