Skip to content

Commit 17acb72

Browse files
ntrrgcntrrgc
committed
[GStreamer] Fix video losing size at the end of the stream
https://bugs.webkit.org/show_bug.cgi?id=219493 Reviewed by Xabier Rodriguez-Calvar. LayoutTests/imported/w3c: Added a test reproducing the bug that gets fixed with the patch. * web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended-expected.txt: Added. * web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended.html: Added. * web-platform-tests/media/test-1s.mp4: Added. * web-platform-tests/media/test-1s.webm: Added. Source/WebCore: 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. 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. Test: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended.html * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfVideo): (WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfAudio): * platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp: (WebCore::TrackPrivateBaseGStreamer::setPad): * platform/graphics/gstreamer/TrackPrivateBaseGStreamer.h: Canonical link: https://commits.webkit.org/232089@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270404 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 3c8c05b commit 17acb72

8 files changed

Lines changed: 97 additions & 4 deletions

File tree

‎LayoutTests/imported/w3c/ChangeLog‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2020-12-03 Alicia Boya García <aboya@igalia.com>
2+
3+
[GStreamer] Fix video losing size at the end of the stream
4+
https://bugs.webkit.org/show_bug.cgi?id=219493
5+
6+
Reviewed by Xabier Rodriguez-Calvar.
7+
8+
Added a test reproducing the bug that gets fixed with the patch.
9+
10+
* web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended-expected.txt: Added.
11+
* web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended.html: Added.
12+
* web-platform-tests/media/test-1s.mp4: Added.
13+
* web-platform-tests/media/test-1s.webm: Added.
14+
115
2020-12-01 Chris Dumez <cdumez@apple.com>
216

317
REGRESSION: imported/w3c/web-platform-tests/streams/transform-streams/terminate.any.html is a flaky failure

‎LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended-expected.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
3+
PASS Video dimensions are preserved at the end of the video.
4+

‎LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended.html‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<!DOCTYPE HTML>
2+
<html>
3+
<head>
4+
<title>HTML5 Media Elements: The size of the video shouldn't be lost after an 'ended' event.</title>
5+
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
6+
<link rel="author" title="Alicia Boya García" href="mailto:aboya@igalia.com"/>
7+
<script src="/resources/testharness.js"></script>
8+
<script src="/resources/testharnessreport.js"></script>
9+
</head>
10+
<body>
11+
<video id="video">
12+
<source src="/media/test-1s.mp4" type="video/mp4">
13+
<source src="/media/test-1s.webm" type="video/webm">
14+
</video>
15+
<script>
16+
promise_test(async (test) => {
17+
const eventWatcher = new EventWatcher(test, video, ["loadedmetadata", "ended"]);
18+
await eventWatcher.wait_for("loadedmetadata");
19+
assert_equals(video.videoWidth, 320, "width when the video is loaded");
20+
assert_equals(video.videoHeight, 240, "height when the video is loaded");
21+
video.play();
22+
await eventWatcher.wait_for(["ended"]);
23+
assert_equals(video.videoWidth, 320, "width after playback");
24+
assert_equals(video.videoHeight, 240, "height after playback");
25+
if (video.videoTracks)
26+
assert_equals(video.videoTracks.length, 1);
27+
}, "Video dimensions are preserved at the end of the video.");
28+
</script>
29+
</body>
30+
</html>

‎LayoutTests/imported/w3c/web-platform-tests/media/test-1s.mp4‎

13.6 KB
Binary file not shown.

‎LayoutTests/imported/w3c/web-platform-tests/media/test-1s.webm‎

22.6 KB
Binary file not shown.

‎Source/WebCore/ChangeLog‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
2020-12-03 Alicia Boya García <aboya@igalia.com>
2+
3+
[GStreamer] Fix video losing size at the end of the stream
4+
https://bugs.webkit.org/show_bug.cgi?id=219493
5+
6+
Reviewed by Xabier Rodriguez-Calvar.
7+
8+
Our port for long had an issue where at the end of the video the
9+
tracks would be erased, causing the video to lose its size and by
10+
extension its aspect ratio.
11+
12+
In absence of a size, WebKit uses the default video size defined by
13+
the spec of 300x150 (2:1 aspect ratio). This causes a video element
14+
that doesn't have a size set through CSS to shrink to that size at the
15+
end of playback, and also for black bars to appear on wider content
16+
(e.g. 16:9 video) when watched in full screen mode.
17+
18+
This patch fixes the problem by not removing the tracks after an end
19+
of stream, and instead reusing them with different pads the next time
20+
the video is played.
21+
22+
Test: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_size_preserved_after_ended.html
23+
24+
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
25+
(WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfVideo):
26+
(WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfAudio):
27+
* platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp:
28+
(WebCore::TrackPrivateBaseGStreamer::setPad):
29+
* platform/graphics/gstreamer/TrackPrivateBaseGStreamer.h:
30+
131
2020-12-03 Aditya Keerthi <akeerthi@apple.com>
232

333
[iOS][FCR] Add new look for buttons in their default state

‎Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,10 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfVideo()
10231023

10241024
ASSERT(m_isLegacyPlaybin || isMediaSource());
10251025

1026+
// Ignore notifications after a EOS. We don't want the tracks to disappear when the video is finished.
1027+
if (m_isEndReached)
1028+
return;
1029+
10261030
unsigned numTracks = 0;
10271031
bool useMediaSource = isMediaSource();
10281032
GstElement* element = useMediaSource ? m_source.get() : m_pipeline.get();
@@ -1056,8 +1060,11 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfVideo()
10561060
RefPtr<VideoTrackPrivateGStreamer> existingTrack = m_videoTracks.get(streamId);
10571061
if (existingTrack) {
10581062
existingTrack->setIndex(i);
1059-
if (existingTrack->pad() == pad)
1060-
continue;
1063+
// If the video has been played twice, the track is still there, but we need
1064+
// to update the pad pointer.
1065+
if (existingTrack->pad() != pad)
1066+
existingTrack->setPad(GRefPtr(pad));
1067+
continue;
10611068
}
10621069
}
10631070

@@ -1101,6 +1108,10 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfAudio()
11011108

11021109
ASSERT(m_isLegacyPlaybin || isMediaSource());
11031110

1111+
// Ignore notifications after a EOS. We don't want the tracks to disappear when the video is finished.
1112+
if (m_isEndReached)
1113+
return;
1114+
11041115
unsigned numTracks = 0;
11051116
bool useMediaSource = isMediaSource();
11061117
GstElement* element = useMediaSource ? m_source.get() : m_pipeline.get();
@@ -1130,8 +1141,11 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfAudio()
11301141
RefPtr<AudioTrackPrivateGStreamer> existingTrack = m_audioTracks.get(streamId);
11311142
if (existingTrack) {
11321143
existingTrack->setIndex(i);
1133-
if (existingTrack->pad() == pad)
1134-
continue;
1144+
// If the video has been played twice, the track is still there, but we need
1145+
// to update the pad pointer.
1146+
if (existingTrack->pad() != pad)
1147+
existingTrack->setPad(GRefPtr(pad));
1148+
continue;
11351149
}
11361150
}
11371151

‎Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class TrackPrivateBaseGStreamer {
4949
};
5050

5151
GstPad* pad() const { return m_pad.get(); }
52+
void setPad(GRefPtr<GstPad>&& pad) { m_pad = WTFMove(pad); }
5253

5354
virtual void disconnect();
5455

0 commit comments

Comments
 (0)