Skip to content

Commit f55d436

Browse files
eocanhaeocanha
committed
[GStreamer][MSE] Support MP3 with ID3 tags
https://bugs.webkit.org/show_bug.cgi?id=291414 Reviewed by Alicia Boya Garcia. If an MP3 with ID3 tags is appended to a SourceBuffer, AppendPipeline's typefind detects it as application/x-id3 instead of audio/mpeg and can't process it. This commit takes care of that case by (optionally) creating an id3demux that takes care of the ID3 tags (just ignores them) and produces audio/mpeg at its output, so the processing can continue normally. We can't know if id3demux can be created or not at AppendPipeline instantiation time. In case the SourceBuffer mimetype is audio/mpeg, we need to listen to the have-type signal from typefind and defer the creation, configuration and connection of the demuxer (or identity, if id3demux is not needed) until the signal happens. The code to configure the demuxer has been refactored into its own method, so it can be reused from the regular and the deferred demuxer configuration codepaths. * LayoutTests/media/content/silence_with_id3_tags.mp3: Added. * LayoutTests/media/media-source/media-source-play-mp3-id3-tag-expected.txt: Added. * LayoutTests/media/media-source/media-source-play-mp3-id3-tag.html: Added. * Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp: (WebCore::AppendPipeline::configureOptionalDemuxerFromAnyThread): Refactored code to configure the demuxer. The configuration code has been enhanced to detect the case of demuxers with static pads (such as id3demux) and treat them in the same immediate configuration way as identity. (WebCore::AppendPipeline::AppendPipeline): If the SourceBuffer mimetype is audio/mpeg, listen to have-type and create and configure the demuxer/identity there (just the first time the signal is triggered). * Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h: Added new configureOptionalDemuxerFromAnyThread() private method. Canonical link: https://commits.webkit.org/294106@main
1 parent 751b9a0 commit f55d436

5 files changed

Lines changed: 150 additions & 34 deletions

File tree

‎LayoutTests/media/content/silence_with_id3_tags.mp3‎

17.7 KB
Binary file not shown.

‎LayoutTests/media/media-source/media-source-play-mp3-id3-tag-expected.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
CONSOLE MESSAGE: Appending MP3 with ID3 tags
2+
CONSOLE MESSAGE: Appended MP3 with ID3 tags. Playing...
3+
CONSOLE MESSAGE: Played. Success!
4+
5+
EXPECTED (v.currentTime > 0 == 'true') OK
6+

‎LayoutTests/media/media-source/media-source-play-mp3-id3-tag.html‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<html>
2+
<head>
3+
<title>Media source play mp3 with ID3 tags</title>
4+
<script src="../video-test.js"></script>
5+
<script src="../utilities.js"></script>
6+
<script>
7+
var v, ms, sb;
8+
9+
async function init()
10+
{
11+
if (window.testRunner)
12+
testRunner.waitUntilDone();
13+
14+
v = document.getElementsByTagName('video')[0];
15+
v.addEventListener('error', function(event) {
16+
console.log("Error on audio element");
17+
if (window.testRunner)
18+
failTest("Error on audio element");
19+
});
20+
21+
ms = new MediaSource();
22+
v.src = URL.createObjectURL(ms);
23+
await once(ms, 'sourceopen');
24+
sb = ms.addSourceBuffer('audio/mpeg');
25+
26+
// Shouldn't happen. Appends must be processed properly.
27+
sb.addEventListener('error', function(event) {
28+
console.log("Error appending");
29+
if (window.testRunner)
30+
failTest("Error appending");
31+
});
32+
33+
console.log("Appending MP3 with ID3 tags");
34+
await Promise.all([fetchAndLoad(sb, '../content/silence_with_id3_tags', [''], '.mp3')]);
35+
ms.endOfStream();
36+
console.log("Appended MP3 with ID3 tags. Playing...");
37+
v.play();
38+
await once(v, 'ended');
39+
testExpected('v.currentTime > 0', true);
40+
41+
console.log("Played. Success!");
42+
43+
if (window.testRunner)
44+
testRunner.notifyDone();
45+
}
46+
</script>
47+
</head>
48+
<body onload="init();">
49+
<video/>
50+
51+
The MP3 file with ID3 tags must be properly appended and played by MSE.
52+
</body>
53+
</html>

‎Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp‎

Lines changed: 90 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include <wtf/Condition.h>
4545
#include <wtf/glib/RunLoopSourcePriority.h>
4646
#include <wtf/text/StringConcatenateNumbers.h>
47+
#include <wtf/text/ASCIILiteral.h>
4748

4849
GST_DEBUG_CATEGORY_EXTERN(webkit_mse_debug);
4950
#define GST_CAT_DEFAULT webkit_mse_debug
@@ -98,6 +99,40 @@ static void assertedElementSetState(GstElement* element, GstState desiredState)
9899
}
99100
}
100101

102+
void AppendPipeline::configureOptionalDemuxerFromAnyThread()
103+
{
104+
ASSERT(m_demux);
105+
106+
auto elementClass = makeString(gst_element_get_metadata(m_demux.get(), GST_ELEMENT_METADATA_KLASS));
107+
// We try to detect special cases of demuxers that have a single static src pad, such as id3demux.
108+
GRefPtr<GstPad> demuxerSrcPad = adoptGRef(gst_element_get_static_pad(m_demux.get(), "src"));
109+
if (!demuxerSrcPad && elementClass.split('/').contains("Demuxer"_s)) {
110+
// These signals won't outlive the lifetime of `this`.
111+
g_signal_connect_swapped(m_demux.get(), "no-more-pads", G_CALLBACK(+[](AppendPipeline* appendPipeline) {
112+
ASSERT(!isMainThread());
113+
GST_DEBUG_OBJECT(appendPipeline->pipeline(), "Posting no-more-pads task to main thread");
114+
appendPipeline->m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([appendPipeline]() {
115+
appendPipeline->didReceiveInitializationSegment();
116+
return AbortableTaskQueue::Void();
117+
});
118+
}), this);
119+
} else {
120+
// m_demux can be an identity or an id3demux element at this point.
121+
gst_pad_add_probe(demuxerSrcPad.get(), GST_PAD_PROBE_TYPE_BUFFER, reinterpret_cast<GstPadProbeCallback>(
122+
+[](GstPad *pad, GstPadProbeInfo*, AppendPipeline* appendPipeline) {
123+
GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(pad));
124+
if (!caps)
125+
return GST_PAD_PROBE_DROP;
126+
appendPipeline->m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([appendPipeline]() {
127+
appendPipeline->didReceiveInitializationSegment();
128+
return AbortableTaskQueue::Void();
129+
});
130+
return GST_PAD_PROBE_REMOVE;
131+
}
132+
), this, nullptr);
133+
}
134+
}
135+
101136
AppendPipeline::AppendPipeline(SourceBufferPrivateGStreamer& sourceBufferPrivate, MediaPlayerPrivateGStreamerMSE& playerPrivate)
102137
: m_sourceBufferPrivate(sourceBufferPrivate)
103138
, m_playerPrivate(&playerPrivate)
@@ -150,47 +185,68 @@ AppendPipeline::AppendPipeline(SourceBufferPrivateGStreamer& sourceBufferPrivate
150185
m_demux = makeGStreamerElement("matroskademux", nullptr);
151186
m_typefind = makeGStreamerElement("identity", nullptr);
152187
} else if (type == "audio/mpeg"_s) {
153-
m_demux = makeGStreamerElement("identity", nullptr);
188+
// Will be instantiated later based on typefind results.
189+
m_demux = nullptr;
154190
m_typefind = makeGStreamerElement("typefind", nullptr);
155-
} else
156-
ASSERT_NOT_REACHED();
157-
158-
#if !LOG_DISABLED
159-
GRefPtr<GstPad> demuxerPad = adoptGRef(gst_element_get_static_pad(m_demux.get(), "sink"));
160-
m_demuxerDataEnteringPadProbeInformation.appendPipeline = this;
161-
m_demuxerDataEnteringPadProbeInformation.description = "demuxer data entering";
162-
m_demuxerDataEnteringPadProbeInformation.probeId = gst_pad_add_probe(demuxerPad.get(), GST_PAD_PROBE_TYPE_BUFFER, reinterpret_cast<GstPadProbeCallback>(appendPipelinePadProbeDebugInformation), &m_demuxerDataEnteringPadProbeInformation, nullptr);
163-
#endif
164191

165-
auto elementClass = makeString(gst_element_get_metadata(m_demux.get(), GST_ELEMENT_METADATA_KLASS));
166-
auto classifiers = elementClass.split('/');
167-
if (classifiers.contains("Demuxer"_s)) {
168-
// These signals won't outlive the lifetime of `this`.
169-
g_signal_connect_swapped(m_demux.get(), "no-more-pads", G_CALLBACK(+[](AppendPipeline* appendPipeline) {
192+
g_signal_connect(m_typefind.get(), "have-type", G_CALLBACK(+[](
193+
GstElement* typefind, guint, GstCaps* caps, AppendPipeline* appendPipeline) {
170194
ASSERT(!isMainThread());
171-
GST_DEBUG_OBJECT(appendPipeline->pipeline(), "Posting no-more-pads task to main thread");
172-
appendPipeline->m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([appendPipeline]() {
173-
appendPipeline->didReceiveInitializationSegment();
174-
return AbortableTaskQueue::Void();
175-
});
195+
196+
// We don't want to create the demuxer twice if the type changes for whatever reason.
197+
if (appendPipeline->m_demux)
198+
return;
199+
200+
auto capsStructure = gst_caps_get_structure(caps, 0);
201+
ASCIILiteral demuxerElementName;
202+
if (gst_structure_has_name(capsStructure, "application/x-id3"))
203+
demuxerElementName = "id3demux"_s;
204+
else if (gst_structure_has_name(capsStructure, "audio/mpeg"))
205+
demuxerElementName = "identity"_s;
206+
207+
if (demuxerElementName.isNull()) {
208+
GST_ELEMENT_ERROR(appendPipeline->pipeline(), STREAM, WRONG_TYPE,
209+
("Unsupported caps for audio/mpeg mimetype: %s",
210+
gst_structure_get_name(capsStructure)), (nullptr));
211+
return;
212+
}
213+
214+
GST_DEBUG_OBJECT(appendPipeline->pipeline(), "Creating %s demuxer for caps: %s",
215+
demuxerElementName.characters(), gst_structure_get_name(capsStructure));
216+
appendPipeline->m_demux = makeGStreamerElement(demuxerElementName.characters(), nullptr);
217+
ASSERT(appendPipeline->m_demux);
218+
219+
appendPipeline->configureOptionalDemuxerFromAnyThread();
220+
221+
// The added element had its floating reference sunk after being assigned to the GRefPtr, so the transfer-floating
222+
// parameter is working as transfer-none here.
223+
gst_bin_add(GST_BIN(GST_ELEMENT_PARENT(typefind)), appendPipeline->m_demux.get());
224+
gst_element_link(appendPipeline->m_typefind.get(), appendPipeline->m_demux.get());
225+
226+
assertedElementSetState(appendPipeline->m_demux.get(), GST_STATE_PLAYING);
176227
}), this);
177228
} else {
178-
GRefPtr<GstPad> identitySrcPad = adoptGRef(gst_element_get_static_pad(m_demux.get(), "src"));
179-
gst_pad_add_probe(identitySrcPad.get(), GST_PAD_PROBE_TYPE_BUFFER, reinterpret_cast<GstPadProbeCallback>(
180-
+[](GstPad *pad, GstPadProbeInfo*, AppendPipeline* appendPipeline) {
181-
GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(pad));
182-
if (!caps)
183-
return GST_PAD_PROBE_DROP;
184-
appendPipeline->m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([appendPipeline]() {
185-
appendPipeline->didReceiveInitializationSegment();
186-
return AbortableTaskQueue::Void();
187-
});
188-
return GST_PAD_PROBE_REMOVE;
189-
}
190-
), this, nullptr);
229+
GST_ELEMENT_ERROR(pipeline(), STREAM, WRONG_TYPE, ("Unsupported container mimetype: %s", type.utf8().data()), (nullptr));
230+
return;
231+
}
232+
233+
// m_demux might be null at this point if there's a typefind pending to identify the proper demuxer to be used
234+
// (see the audio/mpeg case right above).
235+
if (m_demux) {
236+
#if !LOG_DISABLED
237+
GRefPtr<GstPad> demuxerPad = adoptGRef(gst_element_get_static_pad(m_demux.get(), "sink"));
238+
m_demuxerDataEnteringPadProbeInformation.appendPipeline = this;
239+
m_demuxerDataEnteringPadProbeInformation.description = "demuxer data entering";
240+
m_demuxerDataEnteringPadProbeInformation.probeId = gst_pad_add_probe(demuxerPad.get(), GST_PAD_PROBE_TYPE_BUFFER, reinterpret_cast<GstPadProbeCallback>(appendPipelinePadProbeDebugInformation), &m_demuxerDataEnteringPadProbeInformation, nullptr);
241+
#endif
242+
243+
configureOptionalDemuxerFromAnyThread();
191244
}
192245

193-
// Add_many will take ownership of a reference. That's why we used an assignment before.
246+
// The added elements had their floating references sunk after being assigned to the GRefPtr, so the transfer-floating
247+
// parameters are working as transfer-none here.
248+
// Note that m_demux may be null at this point, so the variable argument list would ignore it (m_demux would
249+
// act as a nullptr list guard).
194250
gst_bin_add_many(GST_BIN(m_pipeline.get()), m_appsrc.get(), m_typefind.get(), m_demux.get(), nullptr);
195251
gst_element_link_many(m_appsrc.get(), m_typefind.get(), m_demux.get(), nullptr);
196252

‎Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class AppendPipeline {
102102
bool isLinked() const { return gst_pad_is_linked(entryPad.get()); }
103103
};
104104

105+
void configureOptionalDemuxerFromAnyThread();
105106
void handleErrorSyncMessage(GstMessage*);
106107
void handleNeedContextSyncMessage(GstMessage*);
107108
// For debug purposes only:

0 commit comments

Comments
 (0)