Closed
Bug 876305
Opened 11 years ago
Closed 11 years ago
Media mochitests time out on try with gstreamer backend
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: eflores, Assigned: eflores)
Details
Attachments
(2 files, 1 obsolete file)
1.59 KB,
patch
|
alessandro.d
:
review+
|
Details | Diff | Splinter Review |
4.18 KB,
patch
|
alessandro.d
:
review+
|
Details | Diff | Splinter Review |
The gstreamer backend doesn't use the mime type the server gives us, so the typefind element hammers the MediaResource while trying to infer the type of the stream. This is showing up as timeouts when running media mochitests on try.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #754290 -
Flags: review?(alessandro.d)
Assignee | ||
Updated•11 years ago
|
Summary: Suppress typefind in gstreamer backend → Media mochitests time out on try with gstreamer backend
Assignee | ||
Comment 2•11 years ago
|
||
This patch limits the number of audio buffers used so that we don't decode too much until we're playing. This is to cut down on unnecessary seeking during mochitests which is also causing time outs.
Attachment #754291 -
Flags: review?
Assignee | ||
Updated•11 years ago
|
Attachment #754291 -
Flags: review? → review?(alessandro.d)
Comment 3•11 years ago
|
||
Comment on attachment 754290 [details] [diff] [review] Fix Review of attachment 754290 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/gstreamer/GStreamerReader.h @@ +23,5 @@ > class GStreamerReader : public MediaDecoderReader > { > public: > + GStreamerReader(AbstractMediaDecoder* aDecoder, > + const nsACString& aType); You can get the MIME type from the MediaResource by calling MediaResource::GetContentType(). Please just use that instead of storing it again somewhere else.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #3) > You can get the MIME type from the MediaResource by calling > MediaResource::GetContentType(). Please just use that instead of storing it > again somewhere else. Sigh. I grepped up and down for the string "MIME". Will fix.
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #754290 -
Attachment is obsolete: true
Attachment #754290 -
Flags: review?(alessandro.d)
Attachment #754332 -
Flags: review?(alessandro.d)
Comment 6•11 years ago
|
||
Comment on attachment 754332 [details] [diff] [review] Pass MIME type to gstreamer Review of attachment 754332 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Still depends on https://bugzilla.mozilla.org/show_bug.cgi?id=859199 getting landed first (maybe it shouldn't?). ::: content/media/gstreamer/GStreamerReader.cpp @@ +379,1 @@ > unsupported = !GStreamerFormatHelper::Instance()->CanHandleCodecCaps(caps); I see what this does, but is it related to this specific bug? Just making sure you checked it in intentionally.
Attachment #754332 -
Flags: review?(alessandro.d) → review+
Comment 7•11 years ago
|
||
(In reply to Alessandro Decina from comment #6) > Comment on attachment 754332 [details] [diff] [review] > > unsupported = !GStreamerFormatHelper::Instance()->CanHandleCodecCaps(caps); > > I see what this does, but is it related to this specific bug? Just making > sure you checked it in intentionally. Quoting fail, i meant this (checking for klass Generic): else if (strstr (klass, "Decoder") && !strstr(klass, "Generic"))
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to Alessandro Decina from comment #7) > (In reply to Alessandro Decina from comment #6) > > I see what this does, but is it related to this specific bug? Just making > > sure you checked it in intentionally. > > else if (strstr (klass, "Decoder") && !strstr(klass, > "Generic")) Yeah, when we set the caps on appsrc (video/quicktime), the same caps are set on the decodebin it's connected to. Since decodebin looks like a decoder, this function is trying to match it to the sample types but the sink caps on decodebin are of the container type; so this whole function ends up returning false. I figure generic elements should contain other elements that we will iterate over anyway, so it will be okay. Correct me if I've missed something.
Comment 9•11 years ago
|
||
(In reply to Edwin Flores [:eflores] [:edwin] from comment #8) > Yeah, when we set the caps on appsrc (video/quicktime), the same caps are > set on the decodebin it's connected to. > > Since decodebin looks like a decoder, this function is trying to match it to > the sample types but the sink caps on decodebin are of the container type; > so this whole function ends up returning false. > > I figure generic elements should contain other elements that we will iterate > over anyway, so it will be okay. Correct me if I've missed something. Ah I see, yes you're right.
Comment 10•11 years ago
|
||
Anything still left to be done to land this?
Comment 11•11 years ago
|
||
Comment on attachment 754291 [details] [diff] [review] Limit number of audio buffers used Review of attachment 754291 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I had missed this one. Looks good.
Attachment #754291 -
Flags: review?(alessandro.d) → review+
Assignee | ||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b59b25f6464 https://hg.mozilla.org/integration/mozilla-inbound/rev/8395ce584ddc
Comment 13•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1b59b25f6464 https://hg.mozilla.org/mozilla-central/rev/8395ce584ddc
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•