Closed Bug 876305 Opened 7 years ago Closed 7 years ago

Media mochitests time out on try with gstreamer backend

Categories

(Core :: Audio/Video, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: eflores, Assigned: eflores)

Details

Attachments

(2 files, 1 obsolete file)

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.
Attached patch Fix (obsolete) — Splinter Review
Attachment #754290 - Flags: review?(alessandro.d)
Summary: Suppress typefind in gstreamer backend → Media mochitests time out on try with gstreamer backend
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?
Attachment #754291 - Flags: review? → review?(alessandro.d)
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.
(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.
Attachment #754290 - Attachment is obsolete: true
Attachment #754290 - Flags: review?(alessandro.d)
Attachment #754332 - Flags: review?(alessandro.d)
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+
(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"))
(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.
(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.
Anything still left to be done to land this?
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+
https://hg.mozilla.org/mozilla-central/rev/1b59b25f6464
https://hg.mozilla.org/mozilla-central/rev/8395ce584ddc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.