Bug 204673 – [GStreamer] Convert custom GObject subclasses to WEBKIT_DEFINE_TYPE

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

[Philippe Normand]

Source/WebCore/platform/graphics/gstreamer/GstAllocatorFastMalloc.cpp
40:G_DEFINE_TYPE(GstAllocatorFastMalloc, gst_allocator_fast_malloc, GST_TYPE_ALLOCATOR)

Source/WebCore/platform/graphics/gstreamer/TextCombinerGStreamer.cpp
45:G_DEFINE_TYPE_WITH_CODE(WebKitTextCombiner, webkit_text_combiner, GST_TYPE_BIN,
75:G_DEFINE_TYPE(WebKitTextCombinerPad, webkit_text_combiner_pad, GST_TYPE_GHOST_PAD);

Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp
35:G_DEFINE_TYPE_WITH_CODE(WebKitTextSink, webkit_text_sink, GST_TYPE_APP_SINK,

Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp
141:G_DEFINE_TYPE_WITH_CODE(WebKitVideoSink, webkit_video_sink, GST_TYPE_VIDEO_SINK, GST_DEBUG_CATEGORY_INIT(webkitVideoSinkDebug, "webkitsink", 0, "webkit video sink"));

Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp
65:G_DEFINE_TYPE(WebKitMediaClearKeyDecrypt, webkit_media_clear_key_decrypt, WEBKIT_TYPE_MEDIA_CENC_DECRYPT);

Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
188:G_DEFINE_TYPE_WITH_CODE(WebKitMediaSrc, webkit_media_src, GST_TYPE_BIN,

Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp
60:G_DEFINE_TYPE(WebKitMediaCommonEncryptionDecrypt, webkit_media_common_encryption_decrypt, GST_TYPE_BASE_TRANSFORM);

Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp
136:G_DEFINE_TYPE_WITH_CODE(WebKitWebAudioSrc, webkit_web_audio_src, GST_TYPE_BIN, GST_DEBUG_CATEGORY_INIT(webkit_web_audio_src_debug, \

Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp
332:G_DEFINE_TYPE_WITH_CODE(WebKitMediaStreamSrc, webkit_media_stream_src, GST_TYPE_BIN, doInit);

Source/WebCore/platform/mediastream/libwebrtc/GStreamerVideoEncoder.cpp
92:G_DEFINE_TYPE_WITH_PRIVATE (GstWebrtcVideoEncoder, gst_webrtc_video_encoder,

[Víctor M. Jáquez L.]

Created attachment 407980 [details]
Patch

[Víctor M. Jáquez L.]

I didn't changed GstAllocatorFastMalloc, TextCombinerGStreamer nor TextSinkGStreamer, since they don't use private structures, so there's no case to use the macro. 


GStreamerVideoEncoder it's a bit complex since it doesn't use the private structure as the macro expects it. I'll work on this class later.

GStreamerMediaStreamSource is too complex to me to try to try modify it, perhaps the macro doesn't worth it for it.

[Philippe Normand]

(In reply to Víctor M. Jáquez L. from comment #2)
> I didn't changed GstAllocatorFastMalloc, TextCombinerGStreamer nor
> TextSinkGStreamer, since they don't use private structures, so there's no
> case to use the macro. 
> 
> 
> GStreamerVideoEncoder it's a bit complex since it doesn't use the private
> structure as the macro expects it. I'll work on this class later.
> 

If the intent is still to upstream this to GStreamer it should keep using G_DEFINE_TYPE.

[Philippe Normand]

Comment on attachment 407980 [details]
Patch

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

LGTM, just a nit, valid for all new calls of WEBKIT_DEFINE_TYPE:

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitThunderDecryptorGStreamer.cpp:65
> +WEBKIT_DEFINE_TYPE(WebKitMediaThunderDecrypt, webkit_media_thunder_decrypt, WEBKIT_TYPE_MEDIA_CENC_DECRYPT);

I think there's no need for ending ;

[Víctor M. Jáquez L.]

(In reply to Philippe Normand from comment #3)
> (In reply to Víctor M. Jáquez L. from comment #2)
> > I didn't changed GstAllocatorFastMalloc, TextCombinerGStreamer nor
> > TextSinkGStreamer, since they don't use private structures, so there's no
> > case to use the macro. 
> > 
> > 
> > GStreamerVideoEncoder it's a bit complex since it doesn't use the private
> > structure as the macro expects it. I'll work on this class later.
> > 
> 
> If the intent is still to upstream this to GStreamer it should keep using
> G_DEFINE_TYPE.

I don't think there's an intent to upstream it. I'd need to ask Thibault.

[Víctor M. Jáquez L.]

Created attachment 408170 [details]
Patch

[EWS]

Committed r266697: <https://trac.webkit.org/changeset/266697>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408170 [details].

[Radar WebKit Bug Importer]

<rdar://problem/68455348>

[Carlos Garcia Campos]

Comment on attachment 408170 [details]
Patch

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

> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:84
> +        provider = nullptr;
> +        bus = nullptr;

nullptr initializations aren't needed for private structs, since they are filled with 0 on allocation by GLib.

> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:194
> +    g_type_class_add_private(webKitWebAudioSrcClass, sizeof(WebKitWebAudioSrcPrivate));

This is already called by WEBKIT_DEFINE_TYPE, now it's called twice, which causes a runtime warning.

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:280
>      g_type_class_add_private(klass, sizeof(WebKitVideoSinkPrivate));

You should remove this one too.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitClearKeyDecryptorGStreamer.cpp:88
>      g_type_class_add_private(klass, sizeof(WebKitMediaClearKeyDecryptPrivate));

And this.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:81
>      g_type_class_add_private(klass, sizeof(WebKitMediaCommonEncryptionDecryptPrivate));

And this.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitThunderDecryptorGStreamer.cpp:107
>      g_type_class_add_private(klass, sizeof(WebKitMediaThunderDecryptPrivate));

And this.

[Víctor M. Jáquez L.]

Reopening to attach new patch.

[Víctor M. Jáquez L.]

Created attachment 408220 [details]
Patch

[EWS]

Committed r266721: <https://trac.webkit.org/changeset/266721>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408220 [details].

[Philippe Normand]

https://trac.webkit.org/r266992