[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

Edward Hervey <bilboed> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #272448|none                        |needs-work
             status|                            |

--- Comment #21 from Edward Hervey <bilboed at bilboed.com> 2014-03-20 17:19:00 UTC ---
Review of attachment 272448:
 --> (https://bugzilla.gnome.org/review?bug=675132&attachment=272448)

This is going in the right direction, but should be rebased in a slightly
different way:

* Accumulate buffers that contain SPS and/or PPS and/or SEI (and not use
bytewriters) starting from the first SPS you see
* Accumulate frames starting from the first keyframe
* If you see a new keyframe, flush all previous keyframe/frame
* When you see a frame for your target seek time, you can push out the
requested seek segment on all streams and push out pending buffers

The advantage of this is that you don't need to rewind back to a previous
position. You've already parsed/collected everything you want and can just push
it.

Note that non-h264 streams will also need to have their buffers collected up
until the requested seek position. You can use the stream->pending list for
that.

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

Not needed

@@ +1288,3 @@
       if (G_UNLIKELY (ret != GST_FLOW_OK))
         goto error;
+      GST_DEBUG ("base seek offset set to %" G_GUINT64_FORMAT,

Not needed

::: gst/mpegtsdemux/tsdemux.c
@@ +576,3 @@
+  GstH264NalParser *parser = h264infos->parser;
+
+  if (G_UNLIKELY (parser == NULL)) {

Just initialize everything in one go instead of one by one ? (i.e. parser and
other fields like sps,...)

@@ +738,3 @@
+  GstTsDemuxKeyFrameScanFunction scan_function = NULL;
+
+  if (stream && stream->stream_type == GST_MPEG_TS_STREAM_TYPE_VIDEO_H264) {

Just set all of this once for the stream instead of on every function call ?

@@ +808,2 @@
   if (G_UNLIKELY (start_offset == -1)) {
+    GST_WARNING ("Couldn't convert tart position to an offset");

Wasn't meant to be there ?

@@ +817,3 @@
   res = GST_FLOW_OK;

+  if (flags & GST_SEEK_FLAG_ACCURATE) {

Would make more sense to just store the segment in a separate field
(seek_segment?) and use/adjust it later.

@@ +829,3 @@
   }

+  for (tmp = demux->program->stream_list; tmp; tmp = tmp->next) {

That's not entirely true. You won't need keyframe on audio streams, and you
won't need keyframes on streams you can't handle.

-- 
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.