[GStreamer] Fix video losing size at the end of the stream

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alicia Boya García]

sue where at the end of the video the
tracks would be erased, causing the video to lose its size and by
extension its aspect ratio.

In absence of a size, WebKit uses the default video size defined by
the spec of 300x150 (2:1 aspect ratio). This causes a video element
that doesn't have a size set through CSS to shrink to that size at the
end of playback, and also for black bars to appear on wider content
(e.g. 16:9 video) when watched in full screen mode.

This patch fixes the problem by not removing the tracks after an end
of stream, and instead reusing them with different pads the next time
the video is played.

[Alicia Boya García]

(In reply to Alicia Boya García from comment #0)
> sue where at the end of the video the
> tracks would be erased, causing the video to lose its size and by
> extension its aspect ratio.

The first paragraph got cut:

        Our port for long had an issue where at the end of the video the
        tracks would be erased, causing the video to lose its size and by
        extension its aspect ratio.

[Alicia Boya García]

Created attachment 415305 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Alicia Boya García]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/26743

[Xabier Rodríguez Calvar]

Comment on attachment 415305 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1066
> +                    existingTrack->setPad(GRefPtr(pad));

Nit: Unless that for some reason you want to keep it explicit, you can remove the GRefPtr constructor that will be called implicitly.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1147
> +                    existingTrack->setPad(GRefPtr(pad));

Ditto.

> Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp:179
> +void TrackPrivateBaseGStreamer::setPad(GRefPtr<GstPad>&& pad)
> +{
> +    m_pad = WTFMove(pad);
> +}

Nit: I think you can inline this in the header file.

[Alicia Boya García]

Comment on attachment 415305 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1066
>> +                    existingTrack->setPad(GRefPtr(pad));
> 
> Nit: Unless that for some reason you want to keep it explicit, you can remove the GRefPtr constructor that will be called implicitly.

Unfortunately that's not the case. setPad() expects an rvalue reference, but since here we don't want to do a move, we have to create a new one by coping the reference. Otherwise you get this error:

error: cannot bind rvalue reference of type ‘WTF::GRefPtr<_GstPad>&&’ to lvalue of type ‘WTF::GRefPtr<_GstPad>’

[Xabier Rodríguez Calvar]

Comment on attachment 415305 [details]
Patch

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

>>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1066
>>> +                    existingTrack->setPad(GRefPtr(pad));
>> 
>> Nit: Unless that for some reason you want to keep it explicit, you can remove the GRefPtr constructor that will be called implicitly.
> 
> Unfortunately that's not the case. setPad() expects an rvalue reference, but since here we don't want to do a move, we have to create a new one by coping the reference. Otherwise you get this error:
> 
> error: cannot bind rvalue reference of type ‘WTF::GRefPtr<_GstPad>&&’ to lvalue of type ‘WTF::GRefPtr<_GstPad>’

Ah, sorry, I thought pad was a GstPad* instead of a GRefPtr<GstPad>. Forget this.

[Alicia Boya García]

Created attachment 415325 [details]
Patch for landing

[EWS]

Committed r270404: <https://trac.webkit.org/changeset/270404>

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