Closed Bug 833628 Opened 12 years ago Closed 12 years ago

GStreamer backend has shutdown hang in content/media/ mochitests

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: cpearce, Assigned: alessandro.d)

References

Details

Attachments

(1 file, 1 obsolete file)

Last time I checked, when the gstreamer backend is enabled and the content/media/test mochitests are run we get a shutdown hang. Someone needs to fix this before we can ship GStreamer enabled anywhere.
Alessandro, any chance you can have a look at this? We can't enable GStreamer on Linux without this bug fixed.
Depends on: 806917
Attached patch mp4 fix (obsolete) (deleted) — Splinter Review
This fixes a few mp4 related failures. Not sure if the patch is acceptable as it is or if we want to add a GetLengthNoReallyGetIt() method to MediaResource. Some canPlayType tests still fail, but i'm going to address those in https://bugzilla.mozilla.org/show_bug.cgi?id=760140
Attachment #721614 - Flags: review?
Attachment #721614 - Flags: review? → review?(chris.double)
Attachment #721614 - Attachment is patch: true
Comment on attachment 721614 [details] [diff] [review] mp4 fix Review of attachment 721614 [details] [diff] [review]: ----------------------------------------------------------------- r+ with minor changes. ::: content/media/gstreamer/GStreamerReader.cpp @@ +185,5 @@ > gst_app_src_set_callbacks(mSource, &mSrcCallbacks, (gpointer) this, NULL); > MediaResource* resource = mDecoder->GetResource(); > + > + /* do a short read to trigger a network request so that GetLength() below > + * returns something meaningful an not -1 'and' not 'an' @@ +189,5 @@ > + * returns something meaningful an not -1 > + */ > + char buf[512]; > + unsigned int size = 512; > + resource->Read(buf, size, &size); Make 'size' const and use sizeof buf to compute the value. Or just use sizeof buf in the Read call.
Attachment #721614 - Flags: review?(chris.double) → review+
(In reply to Chris Double (:doublec) from comment #3) > Comment on attachment 721614 [details] [diff] [review] > mp4 fix > > Review of attachment 721614 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ with minor changes. > > ::: content/media/gstreamer/GStreamerReader.cpp > @@ +185,5 @@ > > gst_app_src_set_callbacks(mSource, &mSrcCallbacks, (gpointer) this, NULL); > > MediaResource* resource = mDecoder->GetResource(); > > + > > + /* do a short read to trigger a network request so that GetLength() below > > + * returns something meaningful an not -1 > > 'and' not 'an' Done > > @@ +189,5 @@ > > + * returns something meaningful an not -1 > > + */ > > + char buf[512]; > > + unsigned int size = 512; > > + resource->Read(buf, size, &size); > > Make 'size' const and use sizeof buf to compute the value. Or just use > sizeof buf in the Read call. I can't do that since the last argument is an inout arg set to the number of bytes actually read. I changed it so that size is initialized to 0 and sizeof() is used in the Read() call.
Attached patch fix + review fixes (deleted) — Splinter Review
Attachment #721614 - Attachment is obsolete: true
Comment on attachment 723332 [details] [diff] [review] fix + review fixes Looks good, thanks!
Attachment #723332 - Flags: review+
Assignee: nobody → alessandro.d
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: