[FFmpeg-devel] [PATCH] Fix huge reported DTS on Ogg Theora files with bogus granulepos

[Michael Niedermayer]

On Fri, Jan 13, 2017 at 02:09:00PM -0800, Brion Vibber wrote:
> Patch fixes an issue with broken Ogg Theora videos containing bogus
> negative granulepos on some packets: libavformat was reporting huge
> positive dts timestamps for these packets, causing downstream problems for
> ffmpeg2theora at Wikimedia Commons (near-infinite loop during transcoding).
> 
> Bug report: https://trac.ffmpeg.org/ticket/6080
> Downstream issue: https://phabricator.wikimedia.org/T155247
> 
> Patch for libavformat attached. Returns AV_NOPTS_VALUE instead of bogus
> value for the dts, which downstream ffmpeg2theora can already understand
> and handle.
> 
> Thanks!
> 
> -- brion

>  oggparsetheora.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 42fc64f023a47d8faccf2428a2ef13c0b4d5ac26  0001-Fix-for-dts-on-Ogg-Theora-packets-with-bad-granulepo.patch
> From 8eed58d3f6c2d591e9e179248ff53518270a5363 Mon Sep 17 00:00:00 2001
> From: Brion Vibber <bvibber at wikimedia.org>
> Date: Fri, 13 Jan 2017 13:53:38 -0800
> Subject: [PATCH] Fix for dts on Ogg Theora packets with bad granulepos
> 
> At Wikimedia Commons we found some Ogg Theora video files that had
> packets with bad granulepos (set to -2). This was causing a reported
> dts in the billions of seconds, which triggered further problems
> transcoding with downstream ffmpeg2theora such as recording billions
> of duplicate frames to pad out the time until the bad dts.
> 
> This patch checks for a bad granulepos in theora, similarly to how
> vorbis audio is checked, and returns AV_NOPTS_VALUE for the dts for
> affected packets. That fixes ffmpeg2theora's behavior which was based
> on the dts reported from ffmpeg/libav.
> 
> Downstream bug at Wikimedia: https://phabricator.wikimedia.org/T155247
> ---
>  libavformat/oggparsetheora.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/oggparsetheora.c b/libavformat/oggparsetheora.c
> index b14f9f0669..d0dbe3f2c7 100644
> --- a/libavformat/oggparsetheora.c
> +++ b/libavformat/oggparsetheora.c
> @@ -153,6 +153,10 @@ static uint64_t theora_gptopts(AVFormatContext *ctx, int idx, uint64_t gp,
>      if (!thp)
>          return AV_NOPTS_VALUE;
>  
> +    // Fix for broken files; negative granulepos is invalid.

Please add a reference to the spec and section that says this

is this documented in some specification about ogg or theora ?
I cannot find any definite statment that granulepos is a signed
field, just that -1 as in twos compliment is a special value for it.
and that its interpretation is left to the codec, but i knew that
already.

is something declaring the highest bit to be 0 for all frames of
all codecs in ogg ?

orthogonal to this, timestamps half as large would still cause problems
if software has problems with huge timestamps
We should reject invalid granulepos, if this is invalid but i think
this is not a sufficient fix if the problem is frame duplication
from large timestamp differences

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170116/350e1a0d/attachment.sig>