Bug 191189 – [GStreamer] Move elements registration to GStreamerCommon

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Philippe Normand]

This is only a refactoring.

[Philippe Normand]

Created attachment 353690 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 353690 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:53
> +#if ENABLE(VIDEO)

Shouldn't this enclose more of the includes above?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:-152
> -GST_DEBUG_CATEGORY(webkit_media_player_debug);
> +GST_DEBUG_CATEGORY_EXTERN(webkit_media_player_debug);
>  #define GST_CAT_DEFAULT webkit_media_player_debug
>  
>  
>  namespace WebCore {
>  using namespace std;
>  
> -bool MediaPlayerPrivateGStreamerBase::initializeGStreamerAndRegisterWebKitElements()
> -{
> -    if (!initializeGStreamer())
> -        return false;
> -
> -    static std::once_flag onceFlag;
> -    std::call_once(onceFlag, [] {
> -        GST_DEBUG_CATEGORY_INIT(webkit_media_player_debug, "webkitmediaplayer", 0, "WebKit media player");

This is conceptually weird for me. The category variable should "live" in Base.cpp and be initialized here since it's the superclass. I understand you really want to get rid of this method but IMHO, I think we should keep a very small version of it because of the initialization of this variable, keep the _CATEGORY( here and have the _CATEGORY_EXTERN( in the subclass.

[Philippe Normand]

Comment on attachment 353690 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:53
>> +#if ENABLE(VIDEO)
> 
> Shouldn't this enclose more of the includes above?

I don't think it's necessary because MSE, MediaStream and EME are already declared as depending on VIDEO in WebKitFeatures.cmake.

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:-152
>> -        GST_DEBUG_CATEGORY_INIT(webkit_media_player_debug, "webkitmediaplayer", 0, "WebKit media player");
> 
> This is conceptually weird for me. The category variable should "live" in Base.cpp and be initialized here since it's the superclass. I understand you really want to get rid of this method but IMHO, I think we should keep a very small version of it because of the initialization of this variable, keep the _CATEGORY( here and have the _CATEGORY_EXTERN( in the subclass.

The MSE player already uses its own debug category. Maybe we could have a separate category for the base player after all :) Anyway, for the time being i'll keep a small init function then.

[Philippe Normand]

Created attachment 353712 [details]
Patch

[Philippe Normand]

Committed r237794: <https://trac.webkit.org/changeset/237794>

[Radar WebKit Bug Importer]

<rdar://problem/45802528>