After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778763 - adaptivedemux: Handle seek during preroll
adaptivedemux: Handle seek during preroll
Status: RESOLVED DUPLICATE of bug 696952
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 776606 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-02-16 14:41 UTC by Seungha Yang
Modified: 2018-05-08 07:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adaptivedemux: Add pad probe for upstream event/query (7.47 KB, patch)
2017-02-16 14:45 UTC, Seungha Yang
none Details | Review
adaptivedemux: Print preroll lock/unlock log (3.11 KB, patch)
2017-02-16 14:46 UTC, Seungha Yang
none Details | Review
adaptivedemux: Stop prepare stream's task if needed (2.48 KB, patch)
2017-02-16 14:46 UTC, Seungha Yang
none Details | Review
adaptivedemux: Release manifest lock before pushing flush event (1.61 KB, patch)
2017-02-21 03:15 UTC, Seungha Yang
none Details | Review
adaptivedemux: Reset "cancelled" to FALSE whatever target stream is (1.26 KB, patch)
2017-02-21 04:40 UTC, Seungha Yang
none Details | Review
adaptivedemux: Handle seek event during prerolling (1.25 KB, patch)
2017-02-21 04:40 UTC, Seungha Yang
none Details | Review
hlsdemux: Select target seeking stream depending on demux's preroll state (1.25 KB, patch)
2017-02-21 04:41 UTC, Seungha Yang
none Details | Review
adaptivedemux: Move preroll count to _start_tasks () function (1.74 KB, patch)
2017-02-21 05:36 UTC, Seungha Yang
none Details | Review
adaptivedemux: Move preroll count to _start_tasks () function (1.42 KB, patch)
2017-04-23 15:07 UTC, Seungha Yang
none Details | Review
adaptivedemux: Stop tasks of preroll streams with gst_adaptive_demux_stop_tasks (4.33 KB, patch)
2017-04-23 15:08 UTC, Seungha Yang
none Details | Review
adaptivedemux: Clear pending preroll streams during seek (1.24 KB, patch)
2017-04-23 15:08 UTC, Seungha Yang
none Details | Review
dashdemux: Setup streams again if there are pending preroll streams (1.09 KB, patch)
2017-04-23 15:09 UTC, Seungha Yang
none Details | Review
[DO NOT MERGE] test for adaptivedemux seek during preroll (2.88 KB, patch)
2017-04-23 15:11 UTC, Seungha Yang
none Details | Review

Description Seungha Yang 2017-02-16 14:41:56 UTC
Add probe to fix deadlock and bug on seeking.

* About deadlock
Assume that seek event was forwarded from a srcpad, and demux is handling it now.
At the same time, any queries/event which take manifest lock might be forwarded
from the other pads. Since _handle_seek_event() takes manifest lock,
the queries/event can never be handled until _handle_seek_event() is finished.
Also, demux cannot push flush event since the pads are stream locked by
the queries/event.

* Delay handle seek until preroll done
Because subclass handles seeking only for existing stream (not preparing stream),
wait on preroll.
Comment 1 Seungha Yang 2017-02-16 14:45:50 UTC
Created attachment 345944 [details] [review]
adaptivedemux: Add pad probe for upstream event/query
Comment 2 Seungha Yang 2017-02-16 14:46:07 UTC
Created attachment 345945 [details] [review]
adaptivedemux: Print preroll lock/unlock log
Comment 3 Seungha Yang 2017-02-16 14:46:41 UTC
Created attachment 345946 [details] [review]
adaptivedemux: Stop prepare stream's task if needed

When demux chagnes state from PAUSED to READY, there can be prepared
but not yet exposed streams. To unload pipeline, the tasks of those streams
and corresponding source elements should be stopped.
Comment 4 Nicolas Dufresne (ndufresne) 2017-02-17 02:16:32 UTC
*** Bug 776606 has been marked as a duplicate of this bug. ***
Comment 5 Seungha Yang 2017-02-21 03:15:30 UTC
Created attachment 346297 [details] [review]
adaptivedemux: Release manifest lock before pushing flush event
Comment 6 Seungha Yang 2017-02-21 04:40:35 UTC
Created attachment 346298 [details] [review]
adaptivedemux: Reset "cancelled" to FALSE whatever target stream is

No reason to restrict only non prepared stream.
Comment 7 Seungha Yang 2017-02-21 04:40:56 UTC
Created attachment 346299 [details] [review]
adaptivedemux: Handle seek event during prerolling
Comment 8 Seungha Yang 2017-02-21 04:41:43 UTC
Created attachment 346300 [details] [review]
hlsdemux: Select target seeking stream depending on demux's preroll state

Although demux is doing preroll (not yet finished), demux can get seek event.
In this case, demux needs to do seek only for prepared streams, since existing
streams will be removed after preroll done.
Comment 9 Seungha Yang 2017-02-21 05:36:00 UTC
Created attachment 346302 [details] [review]
adaptivedemux: Move preroll count to _start_tasks () function

To reset preroll pending count whenever start new tasks
Comment 10 Seungha Yang 2017-04-16 02:37:23 UTC
Comment on attachment 346297 [details] [review]
adaptivedemux: Release manifest lock before pushing flush event

Fixed in bug #781320
Comment 11 Edward Hervey 2017-04-16 07:41:02 UTC
Seungha, is the issue still present with that commit from #781320 ?
Comment 12 Seungha Yang 2017-04-16 08:19:17 UTC
(In reply to Edward Hervey from comment #11)
> Seungha, is the issue still present with that commit from #781320 ?

I guess issue still be there but I'll test again on top of master and let you know :) Note that it was hard to reproduce since seek and preroll should be happen at the same time.
Comment 13 Seungha Yang 2017-04-17 04:13:51 UTC
(In reply to Edward Hervey from comment #11)
> Seungha, is the issue still present with that commit from #781320 ?

Hello Edward

The issue is still there... I'll do re-work the previously reported patch and update unit test code for prerolling soon.
Comment 14 Seungha Yang 2017-04-23 15:07:06 UTC
Created attachment 350261 [details] [review]
adaptivedemux: Move preroll count to _start_tasks () function

To reset preroll pending count whenever start new tasks
Comment 15 Seungha Yang 2017-04-23 15:08:17 UTC
Created attachment 350262 [details] [review]
adaptivedemux: Stop tasks of preroll streams with gst_adaptive_demux_stop_tasks

When demux changes state from PAUSED to READY or does seek, there can be prepared
but not yet exposed streams.
Comment 16 Seungha Yang 2017-04-23 15:08:48 UTC
Created attachment 350263 [details] [review]
adaptivedemux: Clear pending preroll streams during seek

Start preroll again if any pending preroll streams are there during seek
Comment 17 Seungha Yang 2017-04-23 15:09:11 UTC
Created attachment 350264 [details] [review]
dashdemux: Setup streams again if there are pending preroll streams

Handle the case that period change and seek happen at the same time
Comment 18 Seungha Yang 2017-04-23 15:11:09 UTC
Created attachment 350265 [details] [review]
[DO NOT MERGE] test for adaptivedemux seek during preroll

- Disable waiting async/buffering to do next action
- Add scenario of seek back/forward for specific multi-period DASH stream
Comment 19 Seungha Yang 2017-04-23 15:12:49 UTC
The issue can be reproduced by

GST_VALIDATE_SCENARIO='seek_over_period' DISPLAY=':0' GST_VALIDATE_OVERRIDE='.../medias/dash-if/DASHIF_TestCases_5c_nomor_4_1a.override' GST_GL_XINITTHREADS='1' .../gst-validate-1.0 playbin uri=http://dash.akamaized.net/dash264/TestCases/5c/nomor/4_1a.mpd audio-sink=fakesink sync=true video-sink=fakesink sync=true qos=true max-lateness=20000000 --set-media-info .../gst-validate/gst-integration-testsuites/medias/dash-if/DASHIF_TestCases_5c_nomor_4_1a.stream_info
Comment 20 Seungha Yang 2017-05-14 04:51:05 UTC
need revisit after fix of bug #773159
Both reports seems to be closely related
Comment 21 Seungha Yang 2017-08-09 03:35:06 UTC
Comment on attachment 350262 [details] [review]
adaptivedemux: Stop tasks of preroll streams with gst_adaptive_demux_stop_tasks

Duplication of bug #785987
Comment 22 Edward Hervey 2018-05-08 05:12:08 UTC
Note that handling seek during preroll (also called "seek-in-ready") is a generic problem that should be solved in another way, I have a plan for that.
Comment 23 Sebastian Dröge (slomo) 2018-05-08 06:52:48 UTC
There is another bug about seeking in ready: https://bugzilla.gnome.org/show_bug.cgi?id=696952
Comment 24 Edward Hervey 2018-05-08 07:30:58 UTC
I explained the logic in that bug. Marking as duplicate.

*** This bug has been marked as a duplicate of bug 696952 ***