GNOME Bugzilla – Bug 580020
[vorbisenc] causes buffers to be out of segment if new segment start time is much greater than 0
Last modified: 2009-05-06 12:15:14 UTC
Fix for #525807 is broken wrt buffers being output that have ts < start when new segment start > 0.
shall we revert it or is there a patch?
A simple fix would be to ensure that the initial segment has the same start time and buffer timestamping as before - that we generate a running time, but add the start time from the first segment.
Created attachment 133427 [details] [review] proposed patch for vorbisenc segment offset Attaching a patch that offsets the running time applied to outgoing buffers by the start time of the first segment so they fall within the input segments. I didn't attempt to make vorbisenc output a single segment when receiving multiple segments, which is probably the correct thing for it to be doing in the longer term.
yes, like theoradec it needs to ouput single segment output and perform clipping on the raw input data.
As far as I can see, theoraenc does *not* do single segment output either - all incoming NEWSEGMENT events are copied to the output verbatim.
yes, you are right, I meant to say that theoraenc also should be changed.
Wim, Zaheer - are you OK with this patch for this release and we do more extensive fixes later?
an ok from me.
Zaheer?
Going to test patch now.
patch seems to work with my test case. so fine from me too.
commit d14c80b22e5d128093fc642cf48406710f9123db Author: Jan Schmidt <thaytan@noraisin.net> Date: Wed Apr 29 16:45:27 2009 +0100 vorbisenc: Ensure output buffers fall within the segment Add the start position of the first segment to the running time used to generate buffer timestamps in vorbisenc. This avoids generating buffers which fall outside the initial segment. The element segment handling requires more extensive fixing, but this at least prevents regressions. Fixes: #580020
gst-launch oggmux name=mux ! filesink location=a.ogg filesrc location=~/videodemo-mpegts-disker.20090428-093332.ts ! mpegtsdemux name=d ! queue max-size-buffers=0 max-size-time=0 ! mad ! audioconvert ! vorbisenc ! queue ! mux. d. ! queue max-size-buffers=0 max-size-time=0 ! mpeg2dec ! ffmpegcolorspace ! theoraenc ! queue ! mux. gives: ogginfo a.ogg Processing file "a.ogg"... New logical stream (#1, serial: 7062f218): type theora New logical stream (#2, serial: 185420ff): type vorbis Vorbis headers parsed for stream 2, information follows... Version: 0 Vendor: Xiph.Org libVorbis I 20080501 Channels: 2 Rate: 48000 Nominal bitrate: 112.000000 kb/s Upper bitrate not set Lower bitrate not set Theora headers parsed for stream 1, information follows... Version: 3.2.1 Vendor: Xiph.Org libTheora I 20081020 3 2 1 Width: 720 Height: 576 Total image: 720 by 576, crop offset (0, 0) Framerate 25/1 (25.00 fps) Pixel aspect ratio 64:45 (1.422222:1) Frame aspect 16:9 Colourspace unspecified Pixel format 4:2:0 Target bitrate: 0 kbps Nominal quality setting (0-63): 16 Warning: EOS not set on stream 1 Theora stream 1: Total data length: 3772476 bytes Playback length: 0m:47.399s Average bitrate: 636.704810 kb/s Warning: EOS not set on stream 2 Vorbis stream 2: Total data length: 0 bytes Playback length: 0m:00.000s Average bitrate: nan kb/s From examining the offsets it looks like they are wrong for vorbisenc also when start time is specified in new segment. So reopening.
I need someone else to fix this, I think - or else it's going to take ages while I learn and understand our hacks around granulepos transport.
Created attachment 133740 [details] [review] use running time not timestamp to compute granulepos like theoraenc does This is in addition to other patch mentioned here (already committed). As a result of the 2 patches, the granulepos is calculated from the running time and the buffer timestamp is running time + initial_ts (gotten from start value of segment).
Patch is not completely correct, it works for test case above but not for the live stream from dvbsrc. I am looking at it now.
Created attachment 134045 [details] [review] use running time not timestamp to compute granulepos like theoraenc does + add clipping of buffers on vorbisenc and theoraenc This attachment adds clipping of buffers to previous one. Also clips on theoraenc, you may want to improve my quickly made (gint64)running_time < 0 check!
Created attachment 134087 [details] [review] use running time not timestamp to compute granulepos like theoraenc does + add clipping of buffers on vorbisenc and theoraenc This patch fixes the obvious memory leak in previous patch and for vorbisenc does sample accurate clipping rather than dropping.
Latest patch looks OK to me :)
commit 1650272b847bf09649853a4a7eeeb1bae12d5e1c Author: Zaheer Merali <zaheerabbas@merali.org> Date: Wed May 6 13:19:34 2009 +0100 vorbisenc, theoraenc: Ensure gp is computed consistently + clip to segment With vorbisenc, compute the granulepos with running time and clip incoming buffers to segment. With theoraenc, drop out of segment buffers.