Bug 167016 - [GStreamer] Several layout tests trigger GStreamer-CRITICAL **: gst_bin_get_by_name: assertion 'GST_IS_BIN (bin)' failed
Summary: [GStreamer] Several layout tests trigger GStreamer-CRITICAL **: gst_bin_get_b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 168588 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-01-13 12:59 PST by Michael Catanzaro
Modified: 2017-02-23 08:30 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.02 KB, patch)
2017-02-22 05:05 PST, Carlos Garcia Campos
calvaris: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2017-01-13 12:59:20 PST
The following layout tests:

http/tests/media/video-accept-encoding.html
http/tests/media/video-query-url.html
http/tests/media/video-redirect.html
http/tests/media/video-served-as-text.html
http/tests/media/video-throttled-load-metadata.html

Have this critical warning printed in stderr output:

GStreamer-CRITICAL **: gst_bin_get_by_name: assertion 'GST_IS_BIN (bin)' failed

The video-redirect.html test has the error printed twice.

The best way to debug such issues is to use the environment variable G_DEBUG=fatal-criticals.
Comment 1 Michael Catanzaro 2017-01-13 13:06:35 PST
Whoops, I missed a lot of affected tests. All of the following tests are affected by this issue:

http/tests/media/video-accept-encoding.html
http/tests/media/video-query-url.html
http/tests/media/video-redirect.html
http/tests/media/video-served-as-text.html
http/tests/media/video-throttled-load-metadata.html
http/tests/security/contentSecurityPolicy/audio-redirect-allowed.html
http/tests/security/contentSecurityPolicy/audio-redirect-allowed2.html
http/tests/security/contentSecurityPolicy/video-with-http-url-allowed-by-csp-media-src-star.html
http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html
http/tests/security/local-video-source-from-remote.html
http/tests/security/video-cross-origin-accessfailure.html
http/tests/security/video-cross-origin-caching.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/media-
media/broken-video.html
media/media-controls-invalid-url.html
media/video-does-not-loop.html
media/video-size.html
Comment 2 Michael Catanzaro 2017-02-20 08:48:04 PST
*** Bug 168588 has been marked as a duplicate of this bug. ***
Comment 3 Michael Catanzaro 2017-02-20 08:48:27 PST
Please refer to bug #168588 for a backtrace to this critical.
Comment 4 Carlos Garcia Campos 2017-02-22 05:05:30 PST
Created attachment 302388 [details]
Patch
Comment 5 Xabier Rodríguez Calvar 2017-02-23 00:56:48 PST
Comment on attachment 302388 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=302388&action=review

> Source/WebCore/ChangeLog:16
> +        (WebCore::MediaPlayerPrivateGStreamer::ensureAudioSourceProvider): Create the AudioSourceProvider is needed.

I guess you mean "if"

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2059
> +void MediaPlayerPrivateGStreamer::ensureAudioSourceProvider()
> +{
> +    if (!m_audioSourceProvider)
> +        m_audioSourceProvider = std::make_unique<AudioSourceProviderGStreamer>();
> +}
> +
> +AudioSourceProvider* MediaPlayerPrivateGStreamer::audioSourceProvider()
> +{
> +    ensureAudioSourceProvider();
> +    return m_audioSourceProvider.get();
> +}

I would probably avoid the ensure function and use audioSourceProvider in all cases above. Besides, it might be interesting to make it inline if possible.

But proposed solution is fine with me too.
Comment 6 Carlos Garcia Campos 2017-02-23 08:30:44 PST
Committed r212889: <http://trac.webkit.org/changeset/212889>