[FFmpeg-devel] [PATCH 2/3] oggdec: verify page checksum

[Mattias Wadman]

On Tue, Apr 28, 2020 at 4:12 PM Lynne <dev at lynne.ee> wrote:
>
> Apr 28, 2020, 14:05 by mattias.wadman at gmail.com:
>
> > On Tue, Apr 28, 2020 at 1:59 PM Lynne <dev at 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.

I see, thanks for the explaining. Also better than in my patch where
it would skip the whole page.

> Anyway, I've implemented checking the version byte after reading the CRC as you suggested
> and have attached the new patch.

Nice, test files works fine now

Would it make sense to conditionally ignore crc mismatch based on
s->error_recognition & AV_EF_CRCCHECK ?

>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".