mirror of
https://gitlab.freedesktop.org/gstreamer/gstreamer.git
synced 2024-12-18 14:26:43 +00:00
oggdemux: fix broken seeking reading the whole file
A change in gst_ogg_demux_do_seek caused oggdemux to wait for a page for each of the streams, including a skeleton stream if one was present. Since Skeleton only has header pages, that was never going to end well. Also, the code was skipping CMML streams when looking for pages, so would also have broken on CMML streams. Thus, we change the code to disregard Skeleton streams, as well as discontinuous streams (such as CMML and Kate). While it may be desirable to consider Kate streams too (in order to avoid losing a subtitle starting near the seek point), this may be a performance drag when seeking where no subtitles are. Maybe one could add a "give up" threshold for such discontinuous streams, so we'd get any page if there is one, but do not end up reading preposterous amounts of data otherwise. In any case, it is important that the code that determines the amount of streams to look pages for remains consistent with the "early out" conditions of the code that actually parses the incoming pages, lest we never decrease the pending counter to zero. This fixes seeking on a file with a skeleton track reading all the file on each seek. https://bugzilla.gnome.org/show_bug.cgi?id=719615
This commit is contained in:
parent
25bf5a13f0
commit
b532ce16a5
1 changed files with 34 additions and 5 deletions
|
@ -2981,11 +2981,40 @@ gst_ogg_demux_do_seek (GstOggDemux * ogg, GstSegment * segment,
|
||||||
&best, FALSE, 0))
|
&best, FALSE, 0))
|
||||||
goto seek_error;
|
goto seek_error;
|
||||||
|
|
||||||
/* second step: find pages for all streams, we use the keyframe_granule to keep
|
/* second step: find pages for all relevant streams. We use the
|
||||||
* track of which ones we saw. If we have seen a page for each stream we can
|
* keyframe_granule to keep track of which ones we saw. If we have
|
||||||
* calculate the positions of each keyframe. */
|
* seen a page for each stream we can calculate the positions of
|
||||||
GST_DEBUG_OBJECT (ogg, "find keyframes");
|
* each keyframe.
|
||||||
|
* Relevant streams are defined as those streams which are not
|
||||||
|
* Skeleton (which only has header pages). Discontinuous streams
|
||||||
|
* such as Kate and CMML are currently excluded, as they could
|
||||||
|
* cause performance issues if there are few pages in the area.
|
||||||
|
* TODO: We might want to include them on a flag, if we want to
|
||||||
|
* not miss a subtitle (Kate has repeat packets for this purpose,
|
||||||
|
* but a stream does not have to use them). */
|
||||||
pending = chain->streams->len;
|
pending = chain->streams->len;
|
||||||
|
for (i = 0; i < chain->streams->len; i++) {
|
||||||
|
GstOggPad *pad = g_array_index (chain->streams, GstOggPad *, i);
|
||||||
|
if (!pad) {
|
||||||
|
GST_WARNING_OBJECT (ogg, "No pad for serialno %08x", pad->map.serialno);
|
||||||
|
pending--;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (pad->map.is_skeleton) {
|
||||||
|
GST_DEBUG_OBJECT (ogg, "Not finding pages for Skeleton stream %08x",
|
||||||
|
pad->map.serialno);
|
||||||
|
pending--;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (pad->map.is_sparse) {
|
||||||
|
GST_DEBUG_OBJECT (ogg, "Not finding pages for sparse stream %08x (%s)",
|
||||||
|
pad->map.serialno, gst_ogg_stream_get_media_type (&pad->map));
|
||||||
|
pending--;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
GST_DEBUG_OBJECT (ogg, "find keyframes for %d/%d streams", pending,
|
||||||
|
chain->streams->len);
|
||||||
|
|
||||||
/* figure out where the keyframes are */
|
/* figure out where the keyframes are */
|
||||||
keytarget = target;
|
keytarget = target;
|
||||||
|
@ -3010,7 +3039,7 @@ gst_ogg_demux_do_seek (GstOggDemux * ogg, GstSegment * segment,
|
||||||
if (pad == NULL)
|
if (pad == NULL)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
if (pad->map.is_skeleton || pad->map.is_cmml)
|
if (pad->map.is_skeleton || pad->map.is_sparse)
|
||||||
goto next;
|
goto next;
|
||||||
|
|
||||||
granulepos = ogg_page_granulepos (&og);
|
granulepos = ogg_page_granulepos (&og);
|
||||||
|
|
Loading…
Reference in a new issue