If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

gstreamer backend hangs on shutdown if decoder is destroyed before seeking

RESOLVED FIXED in mozilla24

Status

()

Core
Audio/Video
RESOLVED FIXED
4 years ago
4 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.
Created attachment 762441 [details] [diff] [review]
Fix
Attachment #762441 - Flags: review?(alessandro.d)
Blocks: 794282
Green (!) on try:
https://tbpl.mozilla.org/?tree=Try&rev=5dc42fa92b2d

Comment 3

4 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-
Created attachment 763344 [details] [diff] [review]
Fix v2

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

4 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/9173dcf026ee
Backed out because of mochitest-1 timeouts on Linux:

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