Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 43c1939

Browse filesBrowse files
thiblahutewebkit-commit-queue
thiblahute
authored and
webkit-commit-queue
committed
Source/WebCore:
[GStreamer] Let playbin handle redirects for us when appropriate https://bugs.webkit.org/show_bug.cgi?id=195326 A simplified mechanism has been added in GStreamer to handle corner cases. Patch by Thibault Saunier <tsaunier@igalia.com> on 2019-10-02 Reviewed by Xabier Rodriguez-Calvar. Not easily testable at our level * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::loadNextLocation): Tools: [GSTreamer] Let playbin handle redirects for us when appropriate https://bugs.webkit.org/show_bug.cgi?id=195326 Added required patche in GStreamer Patch by Thibault Saunier <tsaunier@igalia.com> on 2019-10-02 Reviewed by Xabier Rodriguez-Calvar. * gstreamer/patches/base-0001-playbin-Handle-error-message-with-redirection-indica.patch: Added. * gstreamer/patches/good-0001-qtdemux-Specify-REDIRECT-information-in-error-messag.patch: Added. Canonical link: https://commits.webkit.org/215990@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250624 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent d1df60c commit 43c1939
Copy full SHA for 43c1939

6 files changed

+281-1Lines changed: 281 additions & 1 deletion

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎Source/WebCore/ChangeLog‎

Copy file name to clipboardExpand all lines: Source/WebCore/ChangeLog
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2019-10-02 Thibault Saunier <tsaunier@igalia.com>
2+
3+
[GStreamer] Let playbin handle redirects for us when appropriate
4+
https://bugs.webkit.org/show_bug.cgi?id=195326
5+
6+
A simplified mechanism has been added in GStreamer to handle corner cases.
7+
8+
Reviewed by Xabier Rodriguez-Calvar.
9+
10+
Not easily testable at our level
11+
12+
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
13+
(WebCore::MediaPlayerPrivateGStreamer::loadNextLocation):
14+
115
2019-10-02 Tim Horton <timothy_horton@apple.com>
216

317
Another build fix
Collapse file

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

Copy file name to clipboardExpand all lines: Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
+13-1Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2166,6 +2166,19 @@ bool MediaPlayerPrivateGStreamer::loadNextLocation()
21662166
URL baseUrl = gst_uri_is_valid(newLocation) ? URL() : m_url;
21672167
URL newUrl = URL(baseUrl, newLocation);
21682168

2169+
GUniqueOutPtr<gchar> playbinUrlStr;
2170+
g_object_get(m_pipeline.get(), "current-uri", &playbinUrlStr.outPtr(), nullptr);
2171+
URL playbinUrl(URL(), playbinUrlStr.get());
2172+
2173+
if (playbinUrl == newUrl) {
2174+
GST_DEBUG_OBJECT(pipeline(), "Playbin already handled redirection.");
2175+
2176+
m_url = playbinUrl;
2177+
2178+
return true;
2179+
}
2180+
2181+
changePipelineState(GST_STATE_READY);
21692182
auto securityOrigin = SecurityOrigin::create(m_url);
21702183
if (securityOrigin->canRequest(newUrl)) {
21712184
GST_INFO_OBJECT(pipeline(), "New media url: %s", newUrl.string().utf8().data());
@@ -2178,7 +2191,6 @@ bool MediaPlayerPrivateGStreamer::loadNextLocation()
21782191

21792192
// Reset pipeline state.
21802193
m_resetPipeline = true;
2181-
changePipelineState(GST_STATE_READY);
21822194

21832195
GstState state;
21842196
gst_element_get_state(m_pipeline.get(), &state, nullptr, 0);
Collapse file

‎Tools/ChangeLog‎

Copy file name to clipboardExpand all lines: Tools/ChangeLog
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2019-10-02 Thibault Saunier <tsaunier@igalia.com>
2+
3+
[GSTreamer] Let playbin handle redirects for us when appropriate
4+
https://bugs.webkit.org/show_bug.cgi?id=195326
5+
6+
Added required patche in GStreamer
7+
8+
Reviewed by Xabier Rodriguez-Calvar.
9+
10+
* gstreamer/patches/base-0001-playbin-Handle-error-message-with-redirection-indica.patch: Added.
11+
* gstreamer/patches/good-0001-qtdemux-Specify-REDIRECT-information-in-error-messag.patch: Added.
12+
113
2019-10-01 Aakash Jain <aakash_jain@apple.com>
214

315
[ews] Add API endpoint to retry failed builds for a patch
Collapse file

‎Tools/gstreamer/jhbuild.modules‎

Copy file name to clipboardExpand all lines: Tools/gstreamer/jhbuild.modules
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
<dep package="gstreamer"/>
7070
</dependencies>
7171
<branch hash="sha256:4093aa7b51e28fb24dfd603893fead8d1b7782f088b05ed0f22a21ef176fb5ae" module="gst-plugins-base/gst-plugins-base-${version}.tar.xz" repo="gstreamer" version="1.16.0" >
72+
<patch file="base-0001-playbin-Handle-error-message-with-redirection-indica.patch" strip="1"/>
7273
</branch>
7374
</meson>
7475

@@ -81,6 +82,7 @@
8182
<branch hash="sha256:654adef33380d604112f702c2927574cfc285e31307b79e584113858838bb0fd" module="gst-plugins-good/gst-plugins-good-${version}.tar.xz" repo="gstreamer" version="1.16.0">
8283
<patch file="gst-plugins-good-scaletempo-Advertise-interleaved-layout-in-caps-temp.patch" strip="1"/> <!-- Merged upstream, should be shipped in 1.16.1 -->
8384
<patch file="gst-plugins-good-glibc-2.30-compat.patch" strip="1"/> <!-- Merged upstream, should be shipped in 1.16.1 -->
85+
<patch file="good-0001-qtdemux-Specify-REDIRECT-information-in-error-messag.patch" strip="1"/> <!-- Merged upstream, should be shipped in 1.16.1 -->
8486
</branch>
8587
</meson>
8688

Collapse file

‎Tools/gstreamer/patches/base-0001-playbin-Handle-error-message-with-redirection-indica.patch‎

Copy file name to clipboard
+100Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
From 6e503b6e34000a18a49d2966df2c01c2ef14bd5c Mon Sep 17 00:00:00 2001
2+
From: Thibault Saunier <tsaunier@igalia.com>
3+
Date: Tue, 3 Sep 2019 16:03:49 -0400
4+
Subject: [PATCH] playbin: Handle error message with redirection indication
5+
6+
There are in the wild (mp4) streams that basically contain no tracks
7+
but do have a redirect info[0], in which case, qtdemux won't be able
8+
to expose any pad (there are no tracks) so can't post anything but
9+
an error on the bus, as:
10+
- it can't send EOS downstream, it has no pad,
11+
- posting an EOS message will be useless as PAUSED state can't be
12+
reached and there is no sink in the pipeline meaning GstBin will
13+
simply ignore it
14+
15+
In that case, currently the application could try to handle that but it
16+
is pretty complex as it will get the REDIRECT message on the bus at
17+
which point it could set the URL but playbin will ignore it, as
18+
it will only be for the next EOS, it thus need to set the pipeline to
19+
NULL (READY won't do as it is already in READY at that point). And it
20+
needs to figure out the following ERROR message on the bus needs to be
21+
ignored, which is not really simple.
22+
23+
The approach here is to allow element to add details to the ERROR
24+
message with a `redirect-location` field which elements like playbin handle
25+
and use right away.
26+
27+
We could also use the element 'redirect' message in playbin, but the
28+
issue with that approach is that the element will still emit the ERROR
29+
message on the bus, leading to wrong behaviour. That can't be avoided
30+
since in the case the app/parent pipeline is not handling the redirect
31+
instruction, the ERROR message is necessary (and there is no way to
32+
detect that the message has been "handled" from the element emitting the
33+
redirect).
34+
35+
[0]: http://movietrailers.apple.com/movies/paramount/terminator-dark-fate/terminator-dark-fate-trailer-2_480p.mov
36+
---
37+
gst/playback/gstplaybin2.c | 40 ++++++++++++++++++++++++++++++++++++++
38+
1 file changed, 40 insertions(+)
39+
40+
diff --git a/gst/playback/gstplaybin2.c b/gst/playback/gstplaybin2.c
41+
index c8bfd388e..6997a957c 100644
42+
--- a/gst/playback/gstplaybin2.c
43+
+++ b/gst/playback/gstplaybin2.c
44+
@@ -170,6 +170,10 @@
45+
* type. The new location may be a relative or an absolute URI. Examples
46+
* for such redirects can be found in many quicktime movie trailers.
47+
*
48+
+ * NOTE: playbin will internally handle the redirect messages in the case
49+
+ * that the redirecting stream doesn't contain any tracks and thus
50+
+ * needs to report an error message on the bus.
51+
+ *
52+
* ## Examples
53+
* |[
54+
* gst-launch-1.0 -v playbin uri=file:///path/to/somefile.mp4
55+
@@ -3025,6 +3029,42 @@ gst_play_bin_handle_message (GstBin * bin, GstMessage * msg)
56+
no_more_pads_cb (NULL, group);
57+
}
58+
}
59+
+ } else {
60+
+ const GstStructure *details = NULL;
61+
+
62+
+ gst_message_parse_error_details (msg, &details);
63+
+ if (details && gst_structure_has_field (details, "redirect-location")) {
64+
+ gchar *uri = NULL;
65+
+ const gchar *location =
66+
+ gst_structure_get_string ((GstStructure *) details,
67+
+ "redirect-location");
68+
+
69+
+ if (gst_uri_is_valid (location)) {
70+
+ uri = g_strdup (location);
71+
+ } else {
72+
+ uri = gst_uri_join_strings (group->uri, location);
73+
+ }
74+
+
75+
+ if (g_strcmp0 (uri, group->uri)) {
76+
+ GST_PLAY_BIN_LOCK (playbin);
77+
+ if (playbin->next_group && playbin->next_group->valid) {
78+
+ GST_DEBUG_OBJECT (playbin,
79+
+ "User already setup next uri %s, using it",
80+
+ playbin->next_group->uri);
81+
+ } else {
82+
+ GST_DEBUG_OBJECT (playbin,
83+
+ "Using newly configured redirect URI: %s", uri);
84+
+ gst_play_bin_set_uri (playbin, uri);
85+
+ }
86+
+ GST_PLAY_BIN_UNLOCK (playbin);
87+
+
88+
+ setup_next_source (playbin, GST_STATE_PAUSED);
89+
+ gst_message_unref (msg);
90+
+ msg = NULL;
91+
+ }
92+
+
93+
+ g_free (uri);
94+
+ }
95+
}
96+
}
97+
98+
--
99+
2.21.0
100+
Collapse file

‎Tools/gstreamer/patches/good-0001-qtdemux-Specify-REDIRECT-information-in-error-messag.patch‎

Copy file name to clipboard
+140Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
From a55576d1fd25c7d67661630fc94367908802a496 Mon Sep 17 00:00:00 2001
2+
From: Thibault Saunier <tsaunier@igalia.com>
3+
Date: Tue, 3 Sep 2019 16:46:30 -0400
4+
Subject: [PATCH] qtdemux: Specify REDIRECT information in error message
5+
6+
There are in the wild (mp4) streams that basically contain no tracks
7+
but do have a redirect info[0], in which case, we won't be able
8+
to expose any pad (there are no tracks) so we can't post anything but
9+
an error on the bus, as:
10+
11+
- it can't send EOS downstream, it has no pad,
12+
- posting an EOS message will be useless as PAUSED state can't be
13+
reached and there is no sink in the pipeline meaning GstBin will
14+
simply ignore it
15+
16+
The approach here is to to add details to the ERROR message with a
17+
`redirect-location` field which elements like playbin handle and use right
18+
away.
19+
20+
[0]: http://movietrailers.apple.com/movies/paramount/terminator-dark-fate/terminator-dark-fate-trailer-2_480p.mov
21+
---
22+
gst/isomp4/qtdemux.c | 32 +++++++++++++++++++++++++-------
23+
gst/isomp4/qtdemux.h | 2 +-
24+
2 files changed, 26 insertions(+), 8 deletions(-)
25+
26+
diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c
27+
index ba4d43648..8a6bf08c7 100644
28+
--- a/gst/isomp4/qtdemux.c
29+
+++ b/gst/isomp4/qtdemux.c
30+
@@ -526,6 +526,7 @@ GST_STATIC_PAD_TEMPLATE ("subtitle_%u",
31+
G_DEFINE_TYPE (GstQTDemux, gst_qtdemux, GST_TYPE_ELEMENT);
32+
33+
static void gst_qtdemux_dispose (GObject * object);
34+
+static void gst_qtdemux_finalize (GObject * object);
35+
36+
static guint32
37+
gst_qtdemux_find_index_linear (GstQTDemux * qtdemux, QtDemuxStream * str,
38+
@@ -628,6 +629,7 @@ gst_qtdemux_class_init (GstQTDemuxClass * klass)
39+
parent_class = g_type_class_peek_parent (klass);
40+
41+
gobject_class->dispose = gst_qtdemux_dispose;
42+
+ gobject_class->finalize = gst_qtdemux_finalize;
43+
44+
gstelement_class->change_state = GST_DEBUG_FUNCPTR (gst_qtdemux_change_state);
45+
#if 0
46+
@@ -683,6 +685,16 @@ gst_qtdemux_init (GstQTDemux * qtdemux)
47+
gst_qtdemux_reset (qtdemux, TRUE);
48+
}
49+
50+
+static void
51+
+gst_qtdemux_finalize (GObject * object)
52+
+{
53+
+ GstQTDemux *qtdemux = GST_QTDEMUX (object);
54+
+
55+
+ g_free (qtdemux->redirect_location);
56+
+
57+
+ G_OBJECT_CLASS (parent_class)->finalize (object);
58+
+}
59+
+
60+
static void
61+
gst_qtdemux_dispose (GObject * object)
62+
{
63+
@@ -711,10 +723,11 @@ gst_qtdemux_dispose (GObject * object)
64+
static void
65+
gst_qtdemux_post_no_playable_stream_error (GstQTDemux * qtdemux)
66+
{
67+
- if (qtdemux->posted_redirect) {
68+
- GST_ELEMENT_ERROR (qtdemux, STREAM, DEMUX,
69+
+ if (qtdemux->redirect_location) {
70+
+ GST_ELEMENT_ERROR_WITH_DETAILS (qtdemux, STREAM, DEMUX,
71+
(_("This file contains no playable streams.")),
72+
- ("no known streams found, a redirect message has been posted"));
73+
+ ("no known streams found, a redirect message has been posted"),
74+
+ ("redirect-location", G_TYPE_STRING, qtdemux->redirect_location, NULL));
75+
} else {
76+
GST_ELEMENT_ERROR (qtdemux, STREAM, DEMUX,
77+
(_("This file contains no playable streams.")),
78+
@@ -2111,7 +2124,7 @@ gst_qtdemux_reset (GstQTDemux * qtdemux, gboolean hard)
79+
qtdemux->neededbytes = 16;
80+
qtdemux->todrop = 0;
81+
qtdemux->pullbased = FALSE;
82+
- qtdemux->posted_redirect = FALSE;
83+
+ g_clear_pointer (&qtdemux->redirect_location, g_free);
84+
qtdemux->first_mdat = -1;
85+
qtdemux->header_size = 0;
86+
qtdemux->mdatoffset = -1;
87+
@@ -6065,11 +6078,12 @@ gst_qtdemux_decorate_and_push_buffer (GstQTDemux * qtdemux,
88+
gst_buffer_unmap (buf, &map);
89+
if (url != NULL && strlen (url) != 0) {
90+
/* we have RTSP redirect now */
91+
+ g_free (qtdemux->redirect_location);
92+
+ qtdemux->redirect_location = g_strdup (url);
93+
gst_element_post_message (GST_ELEMENT_CAST (qtdemux),
94+
gst_message_new_element (GST_OBJECT_CAST (qtdemux),
95+
gst_structure_new ("redirect",
96+
"new-location", G_TYPE_STRING, url, NULL)));
97+
- qtdemux->posted_redirect = TRUE;
98+
} else {
99+
GST_WARNING_OBJECT (qtdemux, "Redirect URI of stream is empty, not "
100+
"posting");
101+
@@ -12915,7 +12929,9 @@ qtdemux_expose_streams (GstQTDemux * qtdemux)
102+
"new-location", G_TYPE_STRING,
103+
QTDEMUX_NTH_STREAM (qtdemux, 0)->redirect_uri, NULL));
104+
gst_element_post_message (GST_ELEMENT_CAST (qtdemux), m);
105+
- qtdemux->posted_redirect = TRUE;
106+
+ g_free (qtdemux->redirect_location);
107+
+ qtdemux->redirect_location =
108+
+ g_strdup (QTDEMUX_NTH_STREAM (qtdemux, 0)->redirect_uri);
109+
}
110+
111+
g_ptr_array_foreach (qtdemux->active_streams,
112+
@@ -13968,9 +13984,11 @@ qtdemux_process_redirects (GstQTDemux * qtdemux, GList * references)
113+
g_list_free (references);
114+
115+
GST_INFO_OBJECT (qtdemux, "posting redirect message: %" GST_PTR_FORMAT, s);
116+
+ g_free (qtdemux->redirect_location);
117+
+ qtdemux->redirect_location =
118+
+ g_strdup (gst_structure_get_string (s, "new-location"));
119+
msg = gst_message_new_element (GST_OBJECT_CAST (qtdemux), s);
120+
gst_element_post_message (GST_ELEMENT_CAST (qtdemux), msg);
121+
- qtdemux->posted_redirect = TRUE;
122+
}
123+
124+
/* look for redirect nodes, collect all redirect information and
125+
diff --git a/gst/isomp4/qtdemux.h b/gst/isomp4/qtdemux.h
126+
index f9731b2b8..c5e85c721 100644
127+
--- a/gst/isomp4/qtdemux.h
128+
+++ b/gst/isomp4/qtdemux.h
129+
@@ -69,7 +69,7 @@ struct _GstQTDemux {
130+
/* TRUE if pull-based */
131+
gboolean pullbased;
132+
133+
- gboolean posted_redirect;
134+
+ gchar *redirect_location;
135+
136+
/* Protect pad exposing from flush event */
137+
GMutex expose_lock;
138+
--
139+
2.21.0
140+

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.