-
Notifications
You must be signed in to change notification settings - Fork 152
[2.46][GST]Ensure GST initialized before playing with GST Quirks #1584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: wpe-2.46
Are you sure you want to change the base?
[2.46][GST]Ensure GST initialized before playing with GST Quirks #1584
Conversation
GStreamerQuirksManager verifies each quirk with isPlatformSupported() that usually relies on gst elements presense in the registry. Calling this without gst_init called fails for every gst element and rejects all quirks. The problem exists for apps that don't use any of canPlayType() or isTypeSupported() that handle gst_init internally
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by NOBODY (OOPS!). GStreamerQuirksManager verifies each quirk with isPlatformSupported() that usually relies on gst elements presense in the registry. Calling this without gst_init called fails for every gst element and rejects all quirks. The problem exists for apps that don't use any of canPlayType() or isTypeSupported() that handle gst_init internally. * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Original author: Andrzej Surdej <[email protected]> See: WebPlatformForEmbedded/WPEWebKit#1584
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by NOBODY (OOPS!). GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: WebPlatformForEmbedded/WPEWebKit#1584
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by NOBODY (OOPS!). GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: WebPlatformForEmbedded/WPEWebKit#1584
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by Philippe Normand. GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: WebPlatformForEmbedded/WPEWebKit#1584 Canonical link: https://commits.webkit.org/303982@main
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by Philippe Normand. GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: #1584 Canonical link: https://commits.webkit.org/303982@main
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by Philippe Normand. GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: #1584 Canonical link: https://commits.webkit.org/303982@main
|
This patch introduced crashes in NetworkProcess, which may also use the QuirksManager, because ensureGStreamerInitialized() is only meant for the WebProcess. To solve this I've added an equivalent function for other processes, which just does a minimal initialization of GStreamer. Anyways, this is tracked upstream as https://bugs.webkit.org/show_bug.cgi?id=303326 and landed as WebKit/WebKit@5034d71. I've backported this to 2.46 as dd19f4d and 2.38 as 4bdbd3c. |
|
@vivienne-w Thank you for your help. Could you please provide some more details on the network process crash with my patch? We haven't found that crash while testing on our side and I'm wondering if we missed something |
…gi?id=303326 [GStreamer] Ensure GStreamer is initialized before using the Quirks https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by Philippe Normand. GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: WebPlatformForEmbedded/WPEWebKit#1584 Canonical link: https://commits.webkit.org/303982@main Canonical link: https://commits.webkit.org/298234.313@webkitglib/2.50
|
@vivienne-w I get a WebProcess crash introduced with dd19f4d, at Could you please check? crash backtrace: |
|
@vivienne-w , can you please look at this feedback? Thank you! |
|
@modeveci Taking a look |
|
I'll revert my commit for now, and introduce another one closer to this, just with an extra check to prevent initializing gst in networkprocess @asurdej-comcast I've attached a backtrace from the networkprocess crash in this bug: https://bugs.webkit.org/show_bug.cgi?id=303538 To summarize, QuirkManager may be used from the NetworkProcess when querying MIME type support, and so ensureGStreamerInitialized() would crash since it asserts that it is in the web process |
|
@vivienne-w I've drafted a potential fix here: #1589 as it turns out, wpe-2.46 has both WTF and WebCore runtime application checks. WTF doesn't seem to be used much. |
|
Thanks @vivienne-w , it makes sense now why we don't see that crash. The backtrace that you shared is taken from upstream WebKit. I see there was a recent change for didGetFileInfo() func that calls canShowMIMEType() which requires quirks That change is not part of 2.46, let alone 2.38. |
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by Philippe Normand. GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: #1584 Canonical link: https://commits.webkit.org/303982@main
https://bugs.webkit.org/show_bug.cgi?id=303326 Reviewed by Philippe Normand. GStreamerQuirksManager requires GStreamer to be initialized, which does not happen soon enough if neither canPlayType() nor isTypeSupported() has been called before, so in those cases no quirks are applied. However, the quirks manager may be used in other processes, e.g. NetworkProcess when determining support for a MIME type. To avoid issues, we should only fully init GStreamer if on the WebProcess, and otherwise go for a minimal initialization. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp: (WebCore::ensureGStreamerInitializedNonWebProcess): Added as a counterpart to ensureGStreamerInitialized, only does minimal init. * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h: * Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp: (WebCore::GStreamerRegistryScanner::GStreamerRegistryScanner): Use the new function to initialize gst if in the UIProcess * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Init GStreamer using either of the two methods Original author: Andrzej Surdej <[email protected]> See: #1584 Canonical link: https://commits.webkit.org/303982@main
GStreamerQuirksManager verifies each quirk with isPlatformSupported() that usually relies on gst elements presense in the registry. Calling this without gst_init called fails for every gst element and rejects all quirks.
The problem exists for apps that don't use any of canPlayType() or isTypeSupported() that handle gst_init internally
More details in #1583
cfa5698