Closed Bug 882567 Opened 8 years ago Closed 8 years ago

gstreamer backend hangs on shutdown if decoder is destroyed before seeking

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: eflores, Assigned: eflores)

References

Details

Attachments

(1 file, 1 obsolete file)

Causing mochitest timeouts in content/media/test/test_seek.html; seek10.js.
Attached patch Fix (obsolete) — Splinter Review
Attachment #762441 - Flags: review?(alessandro.d)
Comment on attachment 762441 [details] [diff] [review]
Fix

Review of attachment 762441 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/gstreamer/GStreamerReader.cpp
@@ +362,5 @@
>  
>    *aTags = nullptr;
>  
> +  // Watch the pipeline for fatal errors
> +  gst_bus_add_watch(mBus, GStreamerReader::ErrorCb, this);

gst_bus_add_watch only work if there's a GMainLoop running, which is the case when running under linux, but not in mac. Please use gst_bus_set_sync_handler, which provides the same functionality except the callback is called on pipeline threads instead of being serialized on the main loop. Since the sync handler is called from pipeline threads, you'll need to add locking in the callback (like we do for the preroll and buffer callbacks)
Attachment #762441 - Flags: review?(alessandro.d) → review-
Attached patch Fix v2Splinter Review
Addressed comment 3
Attachment #762441 - Attachment is obsolete: true
Attachment #763344 - Flags: review?(alessandro.d)
(In reply to Alessandro Decina from comment #3)
> Since the sync handler is called from pipeline threads, you'll
> need to add locking in the callback (like we do for the preroll and buffer
> callbacks)

AFAICT we do all the locking we need to in GStreamerReader::Eos()
Comment on attachment 763344 [details] [diff] [review]
Fix v2

Review of attachment 763344 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good
Attachment #763344 - Flags: review?(alessandro.d) → review+
Backed out because of mochitest-1 timeouts on Linux:

https://hg.mozilla.org/integration/mozilla-inbound/rev/822b350df442
https://hg.mozilla.org/mozilla-central/rev/ca547b83df26
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.