Bug 185787 - [GTK][WPE] Start implementing MediaStream API
Summary: [GTK][WPE] Start implementing MediaStream API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 186596 184588 185657
Blocks: 185761
  Show dependency treegraph
 
Reported: 2018-05-18 14:39 PDT by Thibault Saunier
Modified: 2018-06-13 10:11 PDT (History)
9 users (show)

See Also:


Attachments
[GTK][WPE] Start implementing MediaStream API (147.93 KB, patch)
2018-06-01 06:59 PDT, Thibault Saunier
no flags Details | Formatted Diff | Diff
[GTK][WPE]: Implement MediaStream API (149.08 KB, patch)
2018-06-06 15:36 PDT, Thibault Saunier
pnormand: review-
pnormand: commit-queue-
Details | Formatted Diff | Diff
Addressed comments. (146.74 KB, patch)
2018-06-07 07:47 PDT, Thibault Saunier
no flags Details | Formatted Diff | Diff
Fix styling issues. (146.68 KB, patch)
2018-06-07 08:17 PDT, Thibault Saunier
no flags Details | Formatted Diff | Diff
Stop building MockRealtime** from 2 .cmake files. (146.47 KB, patch)
2018-06-07 10:45 PDT, Thibault Saunier
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thibault Saunier 2018-05-18 14:39:48 PDT
This is the following patch after #185761 is merge in order to implement a libwebrtc backend support for the GTK and WPE ports.
Comment 1 Thibault Saunier 2018-06-01 06:59:32 PDT
Created attachment 341758 [details]
[GTK][WPE] Start implementing MediaStream API

This patch implements the MediaStream API for the GTK+ and WPE ports, introducing all the
required classes.

Parts of this patche have already been propose on https://bugs.webkit.org/show_bug.cgi?id=185761
but as discussed there the patch proposal splitting was changed so that the patch introducing new
API to handle enumerateDevices is a patch on its own.

To make sure no previous review is lost in the process, this patch already adresses previous
reviews, and I am replying here to Philip's comment from https://bugs.webkit.org/show_bug.cgi?id=185761:

(In reply to Philippe Normand from comment #5) | from #185761
> Comment on attachment 340841 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340841&action=review
> 
> I did a first pass on the GStreamer bits.
> 
> > Source/WebCore/platform/mediastream/gstreamer/GStreamerAudioCaptureSource.cpp:17
> > + * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
> 
> Hum I don't think we want the Apple version of the header. For every new
> file please make sure to use the standard BSD header.

Fixed on all added files.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerAudioCaptureSource.cpp:135
> > +void GStreamerAudioCaptureSource::stopProducingData()
> 
> This should undo what startProducingData() does I think? Namely disconnect
> the new-sample signal.

Fixed. Same fixed applied to GStreamerVideoCaptureSource::stopProducingData.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerAudioCaptureSource.cpp:142
> > +    if (!m_capabilities) {
> 
> This could become an early return.

Done. Same thing in GStreamerVideoCaptureSource.cpp.
 
> > Source/WebCore/platform/mediastream/gstreamer/GStreamerAudioCapturer.cpp:49
> > +    // FIXME Handle errors.
> 
> Please open a bug and mention it for every FIXME :)

For this one I added an assert, it should not fail otherwise the install is wrong
and playbin expects those elements anyway.

I guess we should go over the FIXME right before merging and do that yes.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerAudioStreamDescription.h:58
> > +        m_platformDescription = { PlatformDescription::GStreamerAudioStreamDescription, (AudioStreamBasicDescription*) &m_info };
> 
> Please avoid C-style casts everywhere in C++ code. Use static_cast or
> reinterpret_cast.

OK, doing that where I noticed it, but ooc, why? :-)

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerAudioStreamDescription.h:83
> > +    double sampleRate() const final { return GST_AUDIO_INFO_RATE (&m_info); }
> 
> No space before ( ... I wonder why check-webkit-style doesn't complain about
> that :/

Fixed.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:141
> > +        ASSERT_NOT_REACHED();
> 
> This is enabled only for debug builds I think. You'd need an early return
> for release builds.

Done.
 
> > Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:172
> > +            // Only accept raw video for now.
> > +            if (!gst_structure_has_name(str, "video/x-raw"))
> 
> Will be nice to support compressed formats too :)

Indeed, not so simple to do but it would be neat :-)
Comment 2 Philippe Normand 2018-06-04 00:40:35 PDT
(In reply to Thibault Saunier from comment #1)
> > 
> > Please avoid C-style casts everywhere in C++ code. Use static_cast or
> > reinterpret_cast.
> 
> OK, doing that where I noticed it, but ooc, why? :-)
> 

This is one of the implicit coding style guidelines of the project :)

> > 
> > Will be nice to support compressed formats too :)
> 
> Indeed, not so simple to do but it would be neat :-)

But that's a requirement for "light" platforms like the RPi IIRC. When I was working on that it was quite challenging to achieve good perf with the RPi Camera without compressed format support.
Comment 3 Philippe Normand 2018-06-04 01:38:43 PDT
Comment on attachment 341758 [details]
[GTK][WPE] Start implementing MediaStream API

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

> LayoutTests/platform/gtk/TestExpectations:-577
> -fast/mediastream [ Skip ]

Can similar test expectation changes be applied to WPE?

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:85
> +void connectSimpleBusMessageCallback(GstElement *pipeline);

* misplaced :)
Also, a disconnect function would probably be nice to have?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:55
> +#include "gstreamer/GStreamerMediaStreamSource.h"

Please add platform/mediastream/gstreamer to the include directories, somewhere in the CMake stuff so there's no need for gstreamer/ prefix here.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1769
> +        // gst_object_ref(sourceElement);

left over?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:85
> +#include "gstreamer/GStreamerMediaStreamSource.h"

No prefix needed.

> Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:54
> +    if (gst_tag_list_get_int(tags, "webkit-media-stream-kind", &kind) && kind == (gint)VideoTrackPrivate::Kind::Main) {

C-style cast detected :)

> Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:56
> +        gst_stream_set_stream_flags(stream.get(), (GstStreamFlags)(streamFlags | GST_STREAM_FLAG_SELECT));

Ditto

> Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDevice.h:42
> +    GstDevice * device() { return m_device.get(); }

no space :P

> Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:110
> +    gst_element_link_many(source.get(), converter.get(), m_capsfilter.get(), m_tee.get(), NULL);

nullptr here too please :)

> Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:178
> +    GRefPtr<GstBus> bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(pipeline())));
> +    gst_bus_set_sync_handler(bus.get(), nullptr, nullptr, nullptr);

As mentioned before, this could go to a new function in GStreamerCommon.

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:32
> +#include "gstreamer/GStreamerAudioData.h"

No WebCore dir prefix in includes, anywhere in this patch please :)

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:37
> +#if GST_CHECK_VERSION(1, 10, 0)

This could be merged with the first #if

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:43
> +#define WEBKIT_MEDIA_STREAM_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass), WEBKIT_TYPE_MEDIA_STREAM_SRC, WebKitMediaStreamSrcClass))
> +#define WEBKIT_IS_MEDIA_STREAM_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), WEBKIT_TYPE_MEDIA_STREAM_SRC))
> +#define WEBKIT_MEDIA_STREAM_SRC_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o), WEBKIT_TYPE_MEDIA_STREAM_SRC, WebKitMediaStreamSrcClass))

I think our other gst custom elements define those macros in their header file.

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:70
> +            (gint)AudioTrackPrivate::Kind::Main, nullptr);

C-style cast. It seems the whole patch needs to be checked about this.

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:164
> +    GstFlowCombiner* flow_combiner;

Variables should use camelCase whenever possible

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:316
> +    /**
> +     * WebKitMediaStreamSrcClass::is-live:
> +     *
> +     * Instruct the source to behave like a live source. This includes that it
> +     * will only push out buffers in the PLAYING state.
> +     */

No need for this, the element is private.

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:319
> +            "Let playbin3 we are a live source.",

+know

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:191
> +            tmpMinFramerate = (int)(tmpMinFPSNumerator / tmpMinFPSDenominator);
> +            tmpMaxFramerate = (int)(tmpMaxFPSNumerator / tmpMaxFPSDenominator);

Framerates are integers in libwebrtc? That's a bit surprising :)
Comment 4 Thibault Saunier 2018-06-06 15:36:15 PDT
Created attachment 342085 [details]
[GTK][WPE]: Implement MediaStream API

(In reply to Philippe Normand from comment #3)
> Comment on attachment 341758 [details]
> [GTK][WPE] Start implementing MediaStream API
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341758&action=review
> 
> > LayoutTests/platform/gtk/TestExpectations:-577
> > -fast/mediastream [ Skip ]
>
> Can similar test expectation changes be applied to WPE?

This will be a follow up patch, we are not ready for that yet but
I am working on it.

> > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:85
> > +void connectSimpleBusMessageCallback(GstElement *pipeline);
> 
> * misplaced :)
> Also, a disconnect function would probably be nice to have?

Added, also fixed a place where we forgot to disconnect (which would have lead to a leak iirc).

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:55
> > +#include "gstreamer/GStreamerMediaStreamSource.h"
> 
> Please add platform/mediastream/gstreamer to the include directories,
> somewhere in the CMake stuff so there's no need for gstreamer/ prefix here.

Done.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1769
> > +        // gst_object_ref(sourceElement);
> 
> left over?

Indeed sorry, FIXED.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:85
> > +#include "gstreamer/GStreamerMediaStreamSource.h"
> 
> No prefix needed.

FIXED.

> > Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:54
> > +    if (gst_tag_list_get_int(tags, "webkit-media-stream-kind", &kind) && kind == (gint)VideoTrackPrivate::Kind::Main) {
> 
> C-style cast detected :)

Fixed
 
> > Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:56
> > +        gst_stream_set_stream_flags(stream.get(), (GstStreamFlags)(streamFlags | GST_STREAM_FLAG_SELECT));
> 
> Ditto

Fixed.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerCaptureDevice.h:42
> > +    GstDevice * device() { return m_device.get(); }
> 
> no space :P

Fixed.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:110
> > +    gst_element_link_many(source.get(), converter.get(), m_capsfilter.get(), m_tee.get(), NULL);
> 
> nullptr here too please :)

Fixed, I wonder how the style checker missed it oO

> 
> > Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:178
> > +    GRefPtr<GstBus> bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(pipeline())));
> > +    gst_bus_set_sync_handler(bus.get(), nullptr, nullptr, nullptr);
> 
> As mentioned before, this could go to a new function in GStreamerCommon.

It is unrelated as this is for the sync message handler.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:32
> > +#include "gstreamer/GStreamerAudioData.h"
> 
> No WebCore dir prefix in includes, anywhere in this patch please :)

Fixed, looks like you spotted the 3 instances :-)

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:37
> > +#if GST_CHECK_VERSION(1, 10, 0)
> 
> This could be merged with the first #if

Problem was that <gst/gst.h> was not included at that point, changed it this way:

```

```

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:43
> > +#define WEBKIT_MEDIA_STREAM_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass), WEBKIT_TYPE_MEDIA_STREAM_SRC, WebKitMediaStreamSrcClass))
> > +#define WEBKIT_IS_MEDIA_STREAM_SRC_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), WEBKIT_TYPE_MEDIA_STREAM_SRC))
> > +#define WEBKIT_MEDIA_STREAM_SRC_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o), WEBKIT_TYPE_MEDIA_STREAM_SRC, WebKitMediaStreamSrcClass))
> 
> I think our other gst custom elements define those macros in their header
> file.

Removed them, they were not used in the end.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:70
> > +            (gint)AudioTrackPrivate::Kind::Main, nullptr);
> 
> C-style cast. It seems the whole patch needs to be checked about this.

Rebuilt the modified c++ files with `-Wold-style-cast` and tried to go over mine (but there were **many** issues from random headers).

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:164
> > +    GstFlowCombiner* flow_combiner;
> 
> Variables should use camelCase whenever possible

Fixed a few I noticed.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:316
> > +    /**
> > +     * WebKitMediaStreamSrcClass::is-live:
> > +     *
> > +     * Instruct the source to behave like a live source. This includes that it
> > +     * will only push out buffers in the PLAYING state.
> > +     */
> 
> No need for this, the element is private.

Removed.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:319
> > +            "Let playbin3 we are a live source.",
> 
> +know
> 
> > Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:191
> > +            tmpMinFramerate = (int)(tmpMinFPSNumerator / tmpMinFPSDenominator);
> > +            tmpMaxFramerate = (int)(tmpMaxFPSNumerator / tmpMaxFPSDenominator);
> 
> Framerates are integers in libwebrtc? That's a bit surprising :)

Crazy thing happen there :P (or we forgot to wake up that day :D).

Fixed.
Comment 5 Philippe Normand 2018-06-07 02:32:08 PDT
Comment on attachment 342085 [details]
[GTK][WPE]: Implement MediaStream API

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

Ok I think this is almost ready to land. Is this patch supposed to apply on trunk? The EWS fails.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:55
> +#include "gstreamer/GStreamerMediaStreamSource.h"

No prefix please ;)

> Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:177
> +    GST_INFO_OBJECT((gpointer) pipeline(), "Going to PLAYING!");

This cast is really needed?

> Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:186
> +    GST_INFO_OBJECT((gpointer) pipeline(), "Tearing down!");

Ditto

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:302
> +    GstElementClass* gstelement_klass = (GstElementClass*)klass;

Use GST_ELEMENT_CLASS(klass)

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:356
> +        (GstPadChainFunction)webkitMediaStreamSrcChain);

You'll think I'm obsessed with casts ;)

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:358
> +    g_assert(gst_element_add_pad(GST_ELEMENT(self), (GstPad*)ghostpad));

GST_PAD_CAST() :)
Also please use ASSERT instead of g_assert
Comment 6 Thibault Saunier 2018-06-07 07:47:43 PDT
Created attachment 342160 [details]
Addressed comments.

(In reply to Philippe Normand from comment #5)
> Comment on attachment 342085 [details]
> [GTK][WPE]: Implement MediaStream API
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=342085&action=review
> 
> Ok I think this is almost ready to land. Is this patch supposed to apply on
> trunk? The EWS fails.

Now that the GstStreamCollection management rework landed, it should apply.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:55
> > +#include "gstreamer/GStreamerMediaStreamSource.h"
> 
> No prefix please ;)

Sorry, FIXED.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:177
> > +    GST_INFO_OBJECT((gpointer) pipeline(), "Going to PLAYING!");
> 
> This cast is really needed?

No, removed.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:186
> > +    GST_INFO_OBJECT((gpointer) pipeline(), "Tearing down!");
> 
> Ditto

Ditto.

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:302
> > +    GstElementClass* gstelement_klass = (GstElementClass*)klass;
> 
> Use GST_ELEMENT_CLASS(klass)
> 
> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:356
> > +        (GstPadChainFunction)webkitMediaStreamSrcChain);
> 
> You'll think I'm obsessed with casts ;)

Fixed. I went over all code manually and tried to fix them all, hopefully I didn't miss
any, but that C macro synthax is hard to detect! (no way to grep it seems).

> > Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:358
> > +    g_assert(gst_element_add_pad(GST_ELEMENT(self), (GstPad*)ghostpad));
> 
> GST_PAD_CAST() :)
> Also please use ASSERT instead of g_assert

Done. Removed all occurences of g_assrt.
Comment 7 Build Bot 2018-06-07 07:50:53 PDT
Attachment 342160 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:25:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerCapturer.cpp:29:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:25:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:29:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:358:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:186:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:187:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:192:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:193:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoCaptureSource.cpp:206:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 11 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Thibault Saunier 2018-06-07 08:17:18 PDT
Created attachment 342168 [details]
Fix styling issues.
Comment 9 Thibault Saunier 2018-06-07 08:19:12 PDT
I had to revert my change for the `#if GST_CHECK_VERSION(1,14,0)` as it requires `gst.h` to be included and including it before local includes is invalid stylewise.
Comment 10 Philippe Normand 2018-06-07 08:38:06 PDT
Comment on attachment 342168 [details]
Fix styling issues.

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

> Source/WebCore/platform/GStreamer.cmake:51
> +        platform/mock/MockRealtimeAudioSource.cpp
> +        platform/mock/MockRealtimeMediaSource.cpp
> +        platform/mock/MockRealtimeMediaSourceCenter.cpp
> +        platform/mock/MockRealtimeVideoSource.cpp

Hum, this should move to a different cmake file I think, they're not related with GStreamer.
Comment 11 Thibault Saunier 2018-06-07 08:54:26 PDT
(In reply to Philippe Normand from comment #10)
> Comment on attachment 342168 [details]
> Fix styling issues.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=342168&action=review
> 
> > Source/WebCore/platform/GStreamer.cmake:51
> > +        platform/mock/MockRealtimeAudioSource.cpp
> > +        platform/mock/MockRealtimeMediaSource.cpp
> > +        platform/mock/MockRealtimeMediaSourceCenter.cpp
> > +        platform/mock/MockRealtimeVideoSource.cpp
> 
> Hum, this should move to a different cmake file I think, they're not related
> with GStreamer.

Where then? Copied twice for WPE and Gtk? They are being built so that we can build the GStreamer wrappers
Comment 12 Philippe Normand 2018-06-07 09:06:39 PDT
Either Source/WebCore/CMakeLists.txt or Source/WebCore/Sources.txt under MEDIA_STREAM guards.
Comment 13 Philippe Normand 2018-06-07 09:08:03 PDT
Actually they're already in Source/WebCore/Sources.txt :)
Comment 14 Thibault Saunier 2018-06-07 10:45:32 PDT
Created attachment 342187 [details]
Stop building MockRealtime** from 2 .cmake files.
Comment 15 WebKit Commit Bot 2018-06-07 11:31:54 PDT
Comment on attachment 342187 [details]
Stop building MockRealtime** from 2 .cmake files.

Clearing flags on attachment: 342187

Committed r232589: <https://trac.webkit.org/changeset/232589>
Comment 16 WebKit Commit Bot 2018-06-07 11:31:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Radar WebKit Bug Importer 2018-06-07 11:32:20 PDT
<rdar://problem/40903578>