oggdemux: fix granpos interpolation violating max keyframe distance

In case many packets fit on a page, we may not see a granpos for
a while, and granpos interpolation can wrap the 'frames since last
keyframe' part of the granpos, generating a granpos which is smaller
than what it should be.

This is fixed by detecting keyframe packets (at least for Theora),
and updating the last keyframe granpos from this.

This may still be generating potentially wrong granpos for streams
which have a Theora like granpos (keyframes, a max keyframe distance
and a count of frames since last keyframe), and which allow implicit
granules on packets. For these streams, a custom keyframe detection
routine should be plugged into their GstOggStream mapper.

https://bugzilla.gnome.org/show_bug.cgi?id=669164

diff --git a/ext/ogg/gstoggdemux.c b/ext/ogg/gstoggdemux.c
index 5baef83..ac01b05 100644 (file)
--- a/ext/ogg/gstoggdemux.c
+++ b/ext/ogg/gstoggdemux.c
@@ -581,6 +581,9 @@ gst_ogg_demux_chain_peer (GstOggPad * pad, ogg_packet * packet,
           pad->current_granule);
     } else if (ogg->segment.rate > 0.0 && pad->current_granule != -1) {
       pad->current_granule += duration;
+      if (gst_ogg_stream_packet_is_key_frame (&pad->map, packet)) {
+        pad->keyframe_granule = pad->current_granule;
+      }
       GST_DEBUG_OBJECT (ogg, "interpolating granule %" G_GUINT64_FORMAT,
           pad->current_granule);
     }

diff --git a/ext/ogg/gstoggstream.c b/ext/ogg/gstoggstream.c
index 0e9debc..3b267fa 100644 (file)
--- a/ext/ogg/gstoggstream.c
+++ b/ext/ogg/gstoggstream.c
@@ -47,9 +47,13 @@ typedef gint64 (*GstOggMapToGranuleposFunc) (GstOggStream * pad,
     gint64 granule, gint64 keyframe_granule);
 
 /* returns TRUE if the granulepos denotes a key frame */
-typedef gboolean (*GstOggMapIsKeyFrameFunc) (GstOggStream * pad,
+typedef gboolean (*GstOggMapIsGranuleposKeyFrameFunc) (GstOggStream * pad,
     gint64 granulepos);
 
+/* returns TRUE if the packet is a key frame */
+typedef gboolean (*GstOggMapIsPacketKeyFrameFunc) (GstOggStream * pad,
+    ogg_packet * packet);
+
 /* returns TRUE if the given packet is a stream header packet */
 typedef gboolean (*GstOggMapIsHeaderPacketFunc) (GstOggStream * pad,
     ogg_packet * packet);
@@ -74,7 +78,8 @@ struct _GstOggMap
   GstOggMapSetupFunc setup_func;
   GstOggMapToGranuleFunc granulepos_to_granule_func;
   GstOggMapToGranuleposFunc granule_to_granulepos_func;
-  GstOggMapIsKeyFrameFunc is_key_frame_func;
+  GstOggMapIsGranuleposKeyFrameFunc is_granulepos_key_frame_func;
+  GstOggMapIsPacketKeyFrameFunc is_packet_key_frame_func;
   GstOggMapIsHeaderPacketFunc is_header_func;
   GstOggMapPacketDurationFunc packet_duration_func;
   GstOggMapGranuleposToKeyGranuleFunc granulepos_to_key_granule_func;
@@ -189,13 +194,25 @@ gst_ogg_stream_granulepos_is_key_frame (GstOggStream * pad, gint64 granulepos)
     return FALSE;
   }
 
-  if (mappers[pad->map].is_key_frame_func == NULL) {
+  if (mappers[pad->map].is_granulepos_key_frame_func == NULL) {
     GST_WARNING ("Failed to determine keyframeness for %s granulepos",
         gst_ogg_stream_get_media_type (pad));
     return FALSE;
   }
 
-  return mappers[pad->map].is_key_frame_func (pad, granulepos);
+  return mappers[pad->map].is_granulepos_key_frame_func (pad, granulepos);
+}
+
+gboolean
+gst_ogg_stream_packet_is_key_frame (GstOggStream * pad, ogg_packet * packet)
+{
+  if (mappers[pad->map].is_packet_key_frame_func == NULL) {
+    GST_WARNING ("Failed to determine keyframeness of %s packet",
+        gst_ogg_stream_get_media_type (pad));
+    return FALSE;
+  }
+
+  return mappers[pad->map].is_packet_key_frame_func (pad, packet);
 }
 
 gboolean
@@ -250,7 +267,13 @@ gst_ogg_stream_get_media_type (GstOggStream * pad)
 /* some generic functions */
 
 static gboolean
-is_keyframe_true (GstOggStream * pad, gint64 granulepos)
+is_granulepos_keyframe_true (GstOggStream * pad, gint64 granulepos)
+{
+  return TRUE;
+}
+
+static gboolean
+is_packet_keyframe_true (GstOggStream * pad, ogg_packet * packet)
 {
   return TRUE;
 }
@@ -390,6 +413,7 @@ setup_theora_mapper (GstOggStream * pad, ogg_packet * packet)
   /* 2 bits + 3 bits = 5 bits KFGSHIFT */
   pad->granuleshift = ((GST_READ_UINT8 (data + 40) & 0x03) << 3) +
       (GST_READ_UINT8 (data + 41) >> 5);
+  GST_LOG ("granshift: %d", pad->granuleshift);
 
   pad->is_video = TRUE;
   pad->n_header_packets = 3;
@@ -446,7 +470,7 @@ granulepos_to_granule_theora (GstOggStream * pad, gint64 granulepos)
 }
 
 static gboolean
-is_keyframe_theora (GstOggStream * pad, gint64 granulepos)
+is_granulepos_keyframe_theora (GstOggStream * pad, gint64 granulepos)
 {
   gint64 frame_mask;
 
@@ -459,6 +483,14 @@ is_keyframe_theora (GstOggStream * pad, gint64 granulepos)
 }
 
 static gboolean
+is_packet_keyframe_theora (GstOggStream * pad, ogg_packet * packet)
+{
+  if (packet->bytes == 0)
+    return FALSE;
+  return (packet->packet[0] & 0xc0) == 0x00;
+}
+
+static gboolean
 is_header_theora (GstOggStream * pad, ogg_packet * packet)
 {
   return (packet->bytes > 0 && (packet->packet[0] & 0x80) == 0x80);
@@ -1957,7 +1989,8 @@ const GstOggMap mappers[] = {
     setup_theora_mapper,
     granulepos_to_granule_theora,
     granule_to_granulepos_default,
-    is_keyframe_theora,
+    is_granulepos_keyframe_theora,
+    is_packet_keyframe_theora,
     is_header_theora,
     packet_duration_constant,
     NULL,
@@ -1969,7 +2002,8 @@ const GstOggMap mappers[] = {
     setup_vorbis_mapper,
     granulepos_to_granule_default,
     granule_to_granulepos_default,
-    is_keyframe_true,
+    is_granulepos_keyframe_true,
+    is_packet_keyframe_true,
     is_header_vorbis,
     packet_duration_vorbis,
     NULL,
@@ -1981,7 +2015,8 @@ const GstOggMap mappers[] = {
     setup_speex_mapper,
     granulepos_to_granule_default,
     granule_to_granulepos_default,
-    is_keyframe_true,
+    is_granulepos_keyframe_true,
+    is_packet_keyframe_true,
     is_header_count,
     packet_duration_constant,
     NULL,
@@ -1994,6 +2029,7 @@ const GstOggMap mappers[] = {
     NULL,
     NULL,
     NULL,
+    NULL,
     is_header_count,
     NULL,
     NULL,
@@ -2006,6 +2042,7 @@ const GstOggMap mappers[] = {
     NULL,
     NULL,
     NULL,
+    NULL,
     is_header_count,
     NULL,
     NULL,
@@ -2018,6 +2055,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_default,
     granule_to_granulepos_default,
     NULL,
+    NULL,
     is_header_count,
     NULL,
     NULL,
@@ -2030,6 +2068,7 @@ const GstOggMap mappers[] = {
     NULL,
     NULL,
     NULL,
+    NULL,
     is_header_true,
     NULL,
     NULL,
@@ -2041,7 +2080,8 @@ const GstOggMap mappers[] = {
     setup_fLaC_mapper,
     granulepos_to_granule_default,
     granule_to_granulepos_default,
-    is_keyframe_true,
+    is_granulepos_keyframe_true,
+    is_packet_keyframe_true,
     is_header_fLaC,
     packet_duration_flac,
     NULL,
@@ -2053,7 +2093,8 @@ const GstOggMap mappers[] = {
     setup_flac_mapper,
     granulepos_to_granule_default,
     granule_to_granulepos_default,
-    is_keyframe_true,
+    is_granulepos_keyframe_true,
+    is_packet_keyframe_true,
     is_header_flac,
     packet_duration_flac,
     NULL,
@@ -2068,6 +2109,7 @@ const GstOggMap mappers[] = {
     NULL,
     NULL,
     NULL,
+    NULL,
     NULL
   },
   {
@@ -2077,6 +2119,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_default,
     granule_to_granulepos_default,
     NULL,
+    NULL,
     is_header_count,
     packet_duration_constant,
     NULL,
@@ -2089,6 +2132,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_default,
     granule_to_granulepos_default,
     NULL,
+    NULL,
     is_header_count,
     packet_duration_kate,
     NULL,
@@ -2101,6 +2145,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_dirac,
     granule_to_granulepos_dirac,
     is_keyframe_dirac,
+    NULL,
     is_header_count,
     packet_duration_constant,
     granulepos_to_key_granule_dirac,
@@ -2113,6 +2158,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_vp8,
     granule_to_granulepos_vp8,
     is_keyframe_vp8,
+    NULL,
     is_header_vp8,
     packet_duration_vp8,
     granulepos_to_key_granule_vp8,
@@ -2125,6 +2171,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_default,
     granule_to_granulepos_default,
     NULL,
+    NULL,
     is_header_opus,
     packet_duration_opus,
     NULL,
@@ -2136,7 +2183,8 @@ const GstOggMap mappers[] = {
     setup_ogmaudio_mapper,
     granulepos_to_granule_default,
     granule_to_granulepos_default,
-    is_keyframe_true,
+    is_granulepos_keyframe_true,
+    is_packet_keyframe_true,
     is_header_ogm,
     packet_duration_ogm,
     NULL,
@@ -2149,6 +2197,7 @@ const GstOggMap mappers[] = {
     granulepos_to_granule_default,
     granule_to_granulepos_default,
     NULL,
+    NULL,
     is_header_ogm,
     packet_duration_constant,
     NULL,
@@ -2160,7 +2209,8 @@ const GstOggMap mappers[] = {
     setup_ogmtext_mapper,
     granulepos_to_granule_default,
     granule_to_granulepos_default,
-    is_keyframe_true,
+    is_granulepos_keyframe_true,
+    is_packet_keyframe_true,
     is_header_ogm,
     packet_duration_ogm,
     NULL,

diff --git a/ext/ogg/gstoggstream.h b/ext/ogg/gstoggstream.h
index a66a78c..c06bc51 100644 (file)
--- a/ext/ogg/gstoggstream.h
+++ b/ext/ogg/gstoggstream.h
@@ -124,6 +124,7 @@ GstClockTime gst_ogg_stream_get_packet_start_time (GstOggStream *pad,
 gboolean gst_ogg_stream_granulepos_is_key_frame (GstOggStream *pad,
     gint64 granulepos);
 gboolean gst_ogg_stream_packet_is_header (GstOggStream *pad, ogg_packet *packet);
+gboolean gst_ogg_stream_packet_is_key_frame (GstOggStream *pad, ogg_packet *packet);
 gint64 gst_ogg_stream_get_packet_duration (GstOggStream * pad, ogg_packet *packet);
 void gst_ogg_stream_extract_tags (GstOggStream * pad, ogg_packet * packet);
 const char *gst_ogg_stream_get_media_type (GstOggStream * pad);