After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 607945 - oggdemux: Ogg Index support for seek in push mode
oggdemux: Ogg Index support for seek in push mode
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal enhancement
: 0.10.30
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 616768 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-01-24 17:02 UTC by Jay L. T. Cornwall
Modified: 2010-08-06 12:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Adds Ogg Skeleton 3.3 parsing support and push mode seeking to oggdemux (18.71 KB, patch)
2010-01-24 17:03 UTC, Jay L. T. Cornwall
none Details | Review
Adds Ogg Skeleton 3.2 parsing support and push mode seeking to oggdemux (18.07 KB, patch)
2010-01-31 19:27 UTC, Jay L. T. Cornwall
none Details | Review
Adds Ogg Skeleton 3.3 parsing support and push mode seeking to oggdemux (22.18 KB, patch)
2010-03-04 12:23 UTC, Jay L. T. Cornwall
none Details | Review

Description Jay L. T. Cornwall 2010-01-24 17:02:45 UTC
Ogg streams present non-trivial problems to an efficient implementation of stream seek in a demuxer. In pull mode, oggdemux performs an initial scan of the entire stream and uses bisection searches to map seek time to stream position. In push mode, there is no support for seeking at all.

The proposed Ogg Index [1] metadata stream rectifies this situation by providing stream statistics and a time:offset index of keypoints in all of the logical streams. This may either be constructed at creation time or appended to an existing Ogg file with a tool such as OggIndex [2]. The proposal is being driven by interest in Ogg Theora as an HTML 5 video format and emerged from a desire to improve its poor seek support, which is currently implemented through bisection search over HTTP.

In the patch which follows, I have constructed a modification of oggdemux to parse the proposed Ogg Index metadata stream, if available, and to use it to implement seeking in push mode. Pull mode is left unchanged. The patch is in an alpha state and is tracking the Ogg Index specification until the format is frozen. Until that time, this patch should be considered a proposal and for testing purposes only.

All comments are welcome. This is my first venture into GStreamer development an I am unfamiliar with the established structure of a demuxer. There may be things that are ugly or just plain broken. I will endeavour to fix any problems in a timely manner.

A test video stream is available at [3]. I have tested the patch in GNOME's Totem media player and in the Rhythmbox player with audio-over-DAAP. Video testing is preliminary but appears to work; I have observed occasional sync-after-seek issues (starting and stopping, as if the audio/video drift is too large) but I believe these are only exposed by this patch and not caused by it. I am happy to be proven wrong. :)

Patch follows in next comment.

[1] http://wiki.xiph.org/Ogg_Index
[2] http://github.com/cpearce/OggIndex
(Note: at the time of writing this tool embeds an incorrect stream frequency in the metadata, causing incorrect playback rates; the author has been notified and a temporary patch is given below.)
[3] http://www.jcornwall.me.uk/temp/big_buck_bunny_427x240.indexed.ogg

diff --git a/src/Decoder.cpp b/src/Decoder.cpp
index 82dd451..6403c7f 100644
--- a/src/Decoder.cpp
+++ b/src/Decoder.cpp
@@ -507,7 +507,7 @@ public:
 
   virtual FisboneInfo GetFisboneInfo() {
     FisboneInfo f;
-    f.mGranNumer = mInfo.channels * mInfo.rate;
+    f.mGranNumer = mInfo.rate;
     f.mGranDenom = 1;
     f.mPreroll = 2;
     f.mGranuleShift = 0;
Comment 1 Jay L. T. Cornwall 2010-01-24 17:03:40 UTC
Created attachment 152158 [details] [review]
Adds Ogg Skeleton 3.3 parsing support and push mode seeking to oggdemux
Comment 2 Sebastian Dröge (slomo) 2010-01-24 17:11:58 UTC
Just two random comments:
- You might want to call the prestime field "pts" :)
- Instead of using a GTree you could use a normal array, sort it after all index points are there (if necessary) and then use gst_util_array_binary_search() on that. That gives you much less allocations and will be faster

Thanks for the patch :)
Comment 3 Jay L. T. Cornwall 2010-01-31 19:27:19 UTC
Created attachment 152690 [details] [review]
Adds Ogg Skeleton 3.2 parsing support and push mode seeking to oggdemux

Here is a new patch for this feature. The main changes are:

* Specification support updated to 3.3. (index compression, no checksums)
* Incorporates Sebastian's suggestions. (thanks!)
* Fixes some formatting issues.

I am investigating poor keyframe hit rates with non-flushing seeks and will post a new patch once fixed.
Comment 4 Jay L. T. Cornwall 2010-03-04 12:23:35 UTC
Created attachment 155224 [details] [review]
Adds Ogg Skeleton 3.3 parsing support and push mode seeking to oggdemux

This patch update addresses poor keyframe hit rates in mixed Ogg Theora/Vorbis streams.

The approach is as follows:
  * Prioritise video keypoints over audio (detected by MIME type) since video keyframes are generally much further apart.
  * Use the keypoint PTS as a substitute for unknown granulepos after a seek, to avoid missing keyframes with no granulepos stamp.
  * Make all other streams resync at their next granulepos because they are likely to be out of sync with the seeked stream, thus we cannot use an estimated PTS.

The few remaining cases of poor hit rate appears to be influenced by the indexing application. Some keypoints point to predictive frames close to image frame, rather than the image frames themselves. This may be intentional or not but does not adversely affect seek quality.
Comment 5 Wim Taymans 2010-05-04 15:41:16 UTC
I had a similar patch laying around that I now commited and enhanced with parts of your code. I will merge some more of your code later.
Comment 6 Jay L. T. Cornwall 2010-05-04 15:45:43 UTC
Hi Wim,

It's worth noting that Skeleton 4.0 will drop shortly and I believe it is structurally incompatible with 3.3 in minor ways (but remains backwards compatible with 3.0).

See: http://github.com/cpearce/OggIndex/blob/master/Skeleton-4.0-Index-Specification.txt (which is itself not frozen until it reaches http://wiki.xiph.org/OggIndex). 4.0 is going into Firefox nightlies so it will probably be the final frozen specification for content creation.
Comment 7 Bastien Nocera 2010-05-06 15:41:03 UTC
*** Bug 616768 has been marked as a duplicate of this bug. ***
Comment 8 Sebastian Dröge (slomo) 2010-06-09 14:40:08 UTC
Is there anything that still needs to be done? We support Skeleton 4 now and seeking in push mode is done through the Skeleton indexes.

Do we want bisection-seeking without indexes in push mode too?
Comment 9 Sebastian Dröge (slomo) 2010-06-09 15:05:36 UTC
Sample file that supports seeking with latest GIT: http://people.freedesktop.org/~slomo/test.indexed.ogg

Bisection seeking would still be nice for push mode ;)
Comment 10 Tim-Philipp Müller 2010-08-06 12:30:19 UTC
Most of this landed in the last release already, didn't it?


> Sample file that supports seeking with latest GIT:
> http://people.freedesktop.org/~slomo/test.indexed.ogg

Works fine in totem, but seeking with the seek example in -base doesn't work so well.


> Bisection seeking would still be nice for push mode ;)

I don't think we need to keep this bug open for that. Feel free to clone it though :)