GNOME Bugzilla – Bug 583903
Ogg muxer or theoraparser fails to remux theora video
Last modified: 2009-08-10 23:32:05 UTC
Using just Vorbis works fine: gst-launch-0.10 filesrc location=tok.ogg ! oggdemux ! vorbisparse ! oggmux ! filesink location=tokremuxed.ogg But if I instead have a file with just theora inside I get this: [cschalle@crazyhorse Videos]$ gst-launch-0.10 filesrc location=theora.ogg ! oggdemux ! theoraparse ! oggmux ! filesink location=theoraremuxed.ogg Setting pipeline to PAUSED ... Pipeline is PREROLLING ... ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed Pipeline is PREROLLED ... Setting pipeline to PLAYING ... New clock: GstSystemClock ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed ** (gst-launch-0.10:19035): CRITICAL **: make_granulepos: assertion `frame >= keyframe' failed
Created attachment 135405 [details] [review] fix
A bit of context: /* If using newer theora, offset the granulepos by +1, see comment * in theora_parse_set_streamheader */ if (!parse->is_old_bitstream) - keyframe += 1; + iframe = keyframe + 1; + else + iframe = keyframe; g_return_val_if_fail (frame >= keyframe, -1); g_return_val_if_fail (frame - keyframe < 1 << parse->shift, -1); - return (keyframe << parse->shift) + (frame - keyframe); + return (iframe << parse->shift) + (frame - keyframe); we can't change the keyframe variable directly. Take as an example when we process the first buffer of a theora stream. The function is called with keyframe == 0, frame == 0 (which are the values of parse->prev_keyframe and parse->prev_frame), since we haven't processed any previous frames. To produce a granulepos compatible with the new bitstream format, we increment the keyframe to have the resulting granulepos == 1|0, which is the correct granulepos for the first frame of a theora stream. The problem is that if we increment keyframe (the variable) directly, then the assertion frame >= keyframe == 0 >= 1 is obviously wrong. The assertion is in fact checking that the _computed_ parse->prev_keyframe and parse->prev_frame are consistent, regardless of the bitstream version. The second assertion checks that the bframe value doesn't overflow the lower bits of granulepos, and is independent of the bitstream format as well.
Looks good to me. I'd recommend adding a comment somewhere that internally theoraparse is using frame numbers indexed from 0, but that _modern_ theora indexes frame numbers from 1 (and old theora from 0).
thanks for the review commit bb1561644f47dcf3991fccb2d8ad64dc766c73f0 Author: Alessandro Decina <alessandro.d@gmail.com> Date: Tue May 26 22:43:34 2009 +0200 theoraparse: fix assertions in make_granulepos when using the new theora granulepos when using the new theora granulepos mapping. Fixes #583903.