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)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: cpearce, Assigned: alessandro.d)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
cajbir
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•12 years ago
|
||
Alessandro, any chance you can have a look at this? We can't enable GStreamer on Linux without this bug fixed.
Assignee | ||
Comment 2•12 years ago
|
||
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?
Assignee | ||
Updated•12 years ago
|
Attachment #721614 -
Flags: review? → review?(chris.double)
Updated•12 years ago
|
Attachment #721614 -
Attachment is patch: true
Comment 3•12 years ago
|
||
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+
Assignee | ||
Comment 4•12 years ago
|
||
(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.
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #721614 -
Attachment is obsolete: true
Comment 6•12 years ago
|
||
Comment on attachment 723332 [details] [diff] [review]
fix + review fixes
Looks good, thanks!
Attachment #723332 -
Flags: review+
Comment 7•12 years ago
|
||
Comment 8•12 years ago
|
||
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.
Description
•