REGRESSION(r280732) [GStreamer] fast/mediastream/getDisplayMedia-max-constraints1.html and other are failing

[Lauro Moura]

fast/mediastream/getDisplayMedia-max-constraints1.html
fast/mediastream/getDisplayMedia-max-constraints2.html

After r280732 (240319@main) (partial revert from r279940)

Both tests fail/time out while waiting for the expectedWidth, which always comes unadjusted for the aspect ratio. e.g. the first test expects 281 for height but only 500 is arriving.

While inspecting the mockvideosource code, indeed the source has its size changed to the expected value, but this doesn't seems to be reflected in the track settings attribute.

[Philippe Normand]

Created attachment 435586 [details]
Patch

[youenn fablet]

Comment on attachment 435586 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        GStreamer ports differ in that regard, in their mock source implementations.

Diverting here is probably not very healthy, though it is true that cocoa port is doing capture out of process and GStreamer is probably not.
Is it only the mock sources that differ or is it also the case for actual sources?
If the issue is with mock, can we realign mocks?

> Source/WebCore/platform/mediastream/RealtimeVideoSource.cpp:119
>      if (size.isEmpty())

Do you know why size is not empty here for those tests in GStreamer but is empty for Cocoa port?

[Philippe Normand]

In GStreamer ports we use our mock video source for mock'ing display capture, while the mac ports use a custom mock display capture source, MockDisplayCapturer (inherits from DisplayCaptureSourceCocoa::Capturer.

For us the size is not empty because it's set from the settings, whereas your Capturer seems to delay that to until the first frame has been generated. I haven't verified this though.

[youenn fablet]

> For us the size is not empty because it's set from the settings

Where is it set in the source code? Does it mirror what the GStreamer capture source is doing?

[Philippe Normand]

This code in the MockRealtimeVideoSource ctor:

    if (mockDisplay()) {
        auto& properties = WTF::get<MockDisplayProperties>(m_device.properties);
        setIntrinsicSize(properties.defaultSize);
        setSize(properties.defaultSize);
        m_fillColor = properties.fillColor;
        return;
    }

[Philippe Normand]

Disabling that setSize call gets me passing tests :P

[Philippe Normand]

(In reply to Philippe Normand from comment #6)
> Disabling that setSize call gets me passing tests :P

Hum no, this needs more debugging...

[Philippe Normand]

Created attachment 435857 [details]
Patch

[youenn fablet]

Comment on attachment 435857 [details]
Patch

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

> Source/WebCore/platform/mediastream/gstreamer/MockRealtimeVideoSourceGStreamer.h:31
>  class MockRealtimeVideoSourceGStreamer final : public MockRealtimeVideoSource {

It seems MockRealtimeVideoSourceGStreamer is only used for display. Should its name be updated? Should it be merged with MockDisplayCaptureSourceGStreamer at some point?

> Source/WebCore/platform/mediastream/gstreamer/MockRealtimeVideoSourceGStreamer.h:47
> +    MockDisplayCaptureSourceGStreamer(RefPtr<MockRealtimeVideoSourceGStreamer>&& source, CaptureDevice::DeviceType type)

Can it be made private? Can we pass a Ref<>&& source.

> Source/WebCore/platform/mediastream/gstreamer/MockRealtimeVideoSourceGStreamer.h:67
> +    RefPtr<MockRealtimeVideoSourceGStreamer> m_source;

Can we make it a Ref<>

[Philippe Normand]

Comment on attachment 435857 [details]
Patch

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

>> Source/WebCore/platform/mediastream/gstreamer/MockRealtimeVideoSourceGStreamer.h:31
>>  class MockRealtimeVideoSourceGStreamer final : public MockRealtimeVideoSource {
> 
> It seems MockRealtimeVideoSourceGStreamer is only used for display. Should its name be updated? Should it be merged with MockDisplayCaptureSourceGStreamer at some point?

It's used for mocking cameras gUM as well.

[Philippe Normand]

Committed r281305 (240725@main): <https://commits.webkit.org/240725@main>

[Radar WebKit Bug Importer]

<rdar://problem/82163904>