[Bug 675132] tsdemux: implement proper seeking with binary search and keyframe detection
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Thu Mar 20 10:13:30 PDT 2014
https://bugzilla.gnome.org/show_bug.cgi?id=675132
GStreamer | gst-plugins-bad | git
--- Comment #22 from Mathieu Duponchelle <mathieu.duponchelle at epitech.eu> 2014-03-20 17:38:12 UTC ---
(In reply to comment #21)
> Review of attachment 272448 [details]:
>
> 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.
We need to rewind anyway to find the first SPS, why would we want to make a
second forward pass when we can get everything in the first pass ?
>
> 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.
up until the requested seek position but from which point ?
>
> ::: 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
>
OK
> ::: 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,...)
>
OK
> @@ +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 ?
>
OK
> @@ +808,2 @@
> if (G_UNLIKELY (start_offset == -1)) {
> + GST_WARNING ("Couldn't convert tart position to an offset");
>
> Wasn't meant to be there ?
>
What do you mean ?
> @@ +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.
>
What would be the reason for that ?
> @@ +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.
OK, I'll discriminate based on the stream content.
--
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.
More information about the gstreamer-bugs
mailing list