833628 - GStreamer backend has shutdown hang in content/media/ mochitests

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Alessandro, any chance you can have a look at this? We can't enable GStreamer on Linux without this bug fixed.

Comment 2

[Alessandro Decina]

Attachment: mp4 fix

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

Comment 3

[cajbir (:cajbir)]

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.

Comment 4

[Alessandro Decina]

(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.

Comment 5

[Alessandro Decina]

Attachment: fix + review fixes

Comment 6

[cajbir (:cajbir)]

Comment on attachment 723332 [details] [diff] [review]
fix + review fixes

Looks good, thanks!

Comment 7

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/baa11ab9988c

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/baa11ab9988c