GNOME Bugzilla – Bug 743900
oggdemux gets first packet timestamp wrong - theora
Last modified: 2015-03-16 12:02:00 UTC
Created attachment 295987 [details] Example file For files which start with a non-zero granulepos, oggdemux outputs the first packet with timestamp 00:00:00. Attaching an example file for which oggdemux correctly calculates a segment of 0:00:02.000000000 - 0:00:03.000000000, but then outputs buffers with 0sec, 2.2sec, 2.4s, 2.6, 2.8.
Created attachment 296144 [details] [review] fix wrong first granule on streams with a hole at start
Review of attachment 296144 [details] [review]: Thanks for the patch! This fixes the timestamping for my test file, but seems to break vorbis timestamping by modifying the duration on the first outgoing packet. Before: /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (30 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7fedcc007220 New clock: GstSystemClock /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (165 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000400 header ) 0x7fedcc007110 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3714 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000400 header ) 0x7fedcc007000 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.000000000, offset: 0, offset_end: 0, flags: 00000000 ) 0x7fedcc007110 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.002902494, offset: 2902494, offset_end: 128, flags: 00000000 ) 0x7fedcc007220 After: /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (30 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7f931c007220 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (165 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000400 header ) 0x7f931c007110 New clock: GstSystemClock /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3714 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000400 header ) 0x7f931c007000 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.013061224, offset: 13061224, offset_end: 576, flags: 00000000 ) 0x7f931c007110 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1 bytes, dts: none, pts: 0:00:00.013061224, duration: 0:00:00.002902494, offset: 15963718, offset_end: 704, flags: 00000000 ) 0x7f931c007220 This causes the decoded vorbis to start 13ms late: /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1024 bytes, dts: none, pts: 0:00:00.013061224, duration: 0:00:00.002902494, offset: -1, offset_end: -1, flags: 00000040 discont ) 0x7f751c0252f0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (1024 bytes, dts: none, pts: 0:00:00.015963718, duration: 0:00:00.002902494, offset: -1, offset_end: -1, flags: 00000000 ) 0x7f752001c6d0
Oops, fixing.
I think this is actually correct. On a sample file here, the first data page contains 30 packets, and has a granpos of 15360. The sum of the durations of the 30 packets is 15360. The first packet has duration... well, this one's a bit odd. The packet itself has duration 128, but oggstream says 0, as it's the first packet in the stream (see ext/ogg/oggstream.c, in packet_duration_vorbis, for the use of pad->last_size). I assume that's per the Vorbis spec. The 15360 total duration is counting that first duration as 0. Now, the first packet (with duration 0) gets a pts of 0, and a duration of 0. The second packet has a pts of 0 too, an a duration of 576-0, 576, which is 0.13ish at 44100 Hz. This appears to be correct to me.
Created attachment 299080 [details] [review] fix wrong first granule on streams with a hole at start I think I got it. Some state in oggstream that was overriding the first packet's duration. I think this is because the first packet initializes the decoder's state. It was correctly set to 0 when accumulating the packet lengths, but then using the duration from the stream when it got to the original code.
That takes care of the issue you found. Please reopen if there's still something. commit ca136e3648d13fd81c54dc0fe7f5d018018a379d Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Feb 4 17:13:44 2015 +0000 oggdemux: fix wrong first granule The code was using the first nonnegative granulepos to seed the granule tracking, which appeared to work since headers have zero granulepos. However, this does not work for files with a hole at start, which are common in live streaming. The correct behavior is to look for the first granule, and subtract the duration of all the packets finishing on this page. The function which does this relies on the fact that the ogg_stream structure can be duplicated by shallow copy, in order to pull the packets from the first page(s) on the copy without affecting the original stream state.
Thanks, I was just about to test that patch. Don't forget to add 'https://bugzilla.gnome.org/show_bug.cgi?id=743900' or so to your commits when you're closing bugs...
Seems to work perfectly. Thanks for hunting it down :)
This breaks playback of short Ogg/Vorbis files like this: https://github.com/sdroege/gst-player/blob/master/tests/media/audio-short.ogg Or also Ogg/Vorbis/theora files that are short: https://github.com/sdroege/gst-player/blob/master/tests/media/audio-video-short.ogg It complains "No valid frames decoded before end of stream" :)
Created attachment 299502 [details] [review] fix regression on streams with clipped start data
This patch fixes the regression on these two files.