gstreamer backend hangs on shutdown if decoder is destroyed before seeking

RESOLVED FIXED in mozilla24

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: eflores, Assigned: eflores)

Tracking

unspecified
mozilla24
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Causing mochitest timeouts in content/media/test/test_seek.html; seek10.js.
Posted patch Fix (obsolete) — Splinter Review
Attachment #762441 - Flags: review?(alessandro.d)

Comment 3

6 years ago
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-
Posted 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 6

6 years ago
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+

Comment 8

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.