[Bug 675132] tsdemux: implement proper seeking with binary search and keyframe detection

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=675132
  GStreamer | gst-plugins-bad | git

--- Comment #27 from Edward Hervey <bilboed at bilboed.com> 2014-03-24 07:24:27 UTC ---
Review of attachment 272581:
 --> (https://bugzilla.gnome.org/review?bug=675132&attachment=272581)

::: gst/mpegtsdemux/mpegtsbase.c
@@ +1105,2 @@
     /* If we don't have enough data, return */
+    if (G_UNLIKELY (pret == PACKET_NEED_MORE)) {

Remove the {} so that this block doesn't end up in the patch.

@@ +1287,3 @@
       if (G_UNLIKELY (ret != GST_FLOW_OK))
         goto error;
+

Remove this. extra line

::: gst/mpegtsdemux/mpegtsbase.h
@@ +114,3 @@
   guint64    seek_offset;

+  /* Used when seeking for a keyframe to go backward in the stream */

This is only used in tsdemux afaics. It should therefore be moved there (and we
end up with no changes in mpegtsbase)

::: gst/mpegtsdemux/tsdemux.c
@@ +793,3 @@
       SEGMENT_ARGS (seeksegment));

+  start = MAX (0, start - SEEK_TIMESTAMP_OFFSET);

This is fine ... but in that case you need to remove the same thing from the
_ts_to_offset() call just below (just use start)

@@ +1411,3 @@
+tsdemux_h264_parsing_info_clear (TSDemuxH264ParsingInfos * h264infos)
+{
+  if (h264infos->sps)

Won't h264infos *always* have all those fields set and present ? If so all the
if() are not needed. And you should reset them to NULL if you reuse this
structure.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.