[Media] Allow access to MediaElement id from MediaPlayerPrivate

https://bugs.webkit.org/show_bug.cgi?id=224818 Patch by Philippe Normand <pnormand@igalia.com> on 2021-04-23 Reviewed by Xabier Rodriguez-Calvar. A new method is added in the MediaPlayer allowing to query the client media element for its identifier. That could be useful for accurate naming of the internal player/pipeline in the MediaPlayerPrivate. If no specific id was set on the media element then the id is empty string and the MediaPlayerPrivate needs to handle that by forging a unique id. This also lead me to simplify pipeline-related code in the GStreamer player. The player can handle only one pipeline in its entire life-time so the code handling pipeline "re-loading" was actually never hit. * html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::parseAttribute): * html/HTMLMediaElement.h: * platform/graphics/MediaPlayer.cpp: (WebCore::MediaPlayer::elementId const): * platform/graphics/MediaPlayer.h: (WebCore::MediaPlayerClient::mediaPlayerElementId const): * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::load): (WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin): (WebCore::MediaPlayerPrivateGStreamer::loadFull): Deleted. (WebCore::MediaPlayerPrivateGStreamer::setPipeline): Deleted. * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: Canonical link: https://commits.webkit.org/236952@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276493 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 23, 2021 · 6db7063 · 6db7063
1 parent 2899877
commit 6db7063
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 39 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
+2021-04-23  Philippe Normand  <pnormand@igalia.com>
+
+        [Media] Allow access to MediaElement id from MediaPlayerPrivate
+        https://bugs.webkit.org/show_bug.cgi?id=224818
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        A new method is added in the MediaPlayer allowing to query the client media element for its
+        identifier. That could be useful for accurate naming of the internal player/pipeline in the
+        MediaPlayerPrivate. If no specific id was set on the media element then the id is empty
+        string and the MediaPlayerPrivate needs to handle that by forging a unique id.
+
+        This also lead me to simplify pipeline-related code in the GStreamer player. The player can
+        handle only one pipeline in its entire life-time so the code handling pipeline "re-loading"
+        was actually never hit.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute):
+        * html/HTMLMediaElement.h:
+        * platform/graphics/MediaPlayer.cpp:
+        (WebCore::MediaPlayer::elementId const):
+        * platform/graphics/MediaPlayer.h:
+        (WebCore::MediaPlayerClient::mediaPlayerElementId const):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::load):
+        (WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin):
+        (WebCore::MediaPlayerPrivateGStreamer::loadFull): Deleted.
+        (WebCore::MediaPlayerPrivateGStreamer::setPipeline): Deleted.
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+
 2021-04-22  Sergio Villar Senin  <svillar@igalia.com>
 
         Do not set synchronous scrolling for layers without relevant scrolling scopes

diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -712,6 +712,9 @@ bool HTMLMediaElement::isInteractiveContent() const
 
 void HTMLMediaElement::parseAttribute(const QualifiedName& name, const AtomString& value)
 {
+    if (name == idAttr)
+        m_id = value;
+
     if (name == srcAttr) {
         // https://html.spec.whatwg.org/multipage/embedded-content.html#location-of-the-media-resource
         // Location of the Media Resource

diff --git a/Source/WebCore/html/HTMLMediaElement.h b/Source/WebCore/html/HTMLMediaElement.h
@@ -725,6 +725,8 @@ class HTMLMediaElement
     String mediaPlayerSourceApplicationIdentifier() const override { return sourceApplicationIdentifier(); }
     Vector<String> mediaPlayerPreferredAudioCharacteristics() const override;
 
+    String mediaPlayerElementId() const override { return m_id; }
+
 #if PLATFORM(IOS_FAMILY)
     String mediaPlayerNetworkInterfaceName() const override;
     void mediaPlayerGetRawCookies(const URL&, MediaPlayerClient::GetRawCookiesCallback&&) const final;
@@ -1199,6 +1201,7 @@ class HTMLMediaElement
 #if ENABLE(MEDIA_STREAM)
     String m_audioOutputHashedDeviceId;
 #endif
+    String m_id;
 };
 
 String convertEnumerationToString(HTMLMediaElement::AutoplayEventPlaybackState);

diff --git a/Source/WebCore/platform/graphics/MediaPlayer.cpp b/Source/WebCore/platform/graphics/MediaPlayer.cpp
@@ -1669,6 +1669,11 @@ MediaPlayerIdentifier MediaPlayer::identifier() const
     return m_private->identifier();
 }
 
+String MediaPlayer::elementId() const
+{
+    return client().mediaPlayerElementId();
+}
+
 #if !RELEASE_LOG_DISABLED
 const Logger& MediaPlayer::mediaPlayerLogger()
 {

diff --git a/Source/WebCore/platform/graphics/MediaPlayer.h b/Source/WebCore/platform/graphics/MediaPlayer.h
@@ -258,6 +258,8 @@ class MediaPlayerClient {
 
     virtual String mediaPlayerSourceApplicationIdentifier() const { return emptyString(); }
 
+    virtual String mediaPlayerElementId() const { return emptyString(); }
+
     virtual void mediaPlayerEngineFailedToLoad() const { }
 
     virtual double mediaPlayerRequestedPlaybackRate() const { return 0; }
@@ -549,6 +551,8 @@ class WEBCORE_EXPORT MediaPlayer : public MediaPlayerEnums, public ThreadSafeRef
     String engineDescription() const;
     long platformErrorCode() const;
 
+    String elementId() const;
+
     CachedResourceLoader* cachedResourceLoader();
     RefPtr<PlatformMediaResourceLoader> createResourceLoader();
 

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
@@ -302,7 +302,7 @@ void MediaPlayerPrivateGStreamer::registerMediaEngine(MediaEngineRegistrar regis
     registrar(makeUnique<MediaPlayerFactoryGStreamer>());
 }
 
-void MediaPlayerPrivateGStreamer::loadFull(const String& urlString, const String& pipelineName)
+void MediaPlayerPrivateGStreamer::load(const String& urlString)
 {
     URL url(URL(), urlString);
     if (url.protocolIsAbout()) {
@@ -318,7 +318,7 @@ void MediaPlayerPrivateGStreamer::loadFull(const String& urlString, const String
     registerWebKitGStreamerElements();
 
     if (!m_pipeline)
-        createGSTPlayBin(url, pipelineName);
+        createGSTPlayBin(url);
     syncOnClock(true);
     if (m_fillTimer.isActive())
         m_fillTimer.stop();
@@ -346,11 +346,6 @@ void MediaPlayerPrivateGStreamer::loadFull(const String& urlString, const String
         commitLoad();
 }
 
-void MediaPlayerPrivateGStreamer::load(const String& urlString)
-{
-    loadFull(urlString, String());
-}
-
 #if ENABLE(MEDIA_SOURCE)
 void MediaPlayerPrivateGStreamer::load(const URL&, const ContentType&, MediaSourcePrivateClient*)
 {
@@ -364,10 +359,7 @@ void MediaPlayerPrivateGStreamer::load(const URL&, const ContentType&, MediaSour
 void MediaPlayerPrivateGStreamer::load(MediaStreamPrivate& stream)
 {
     m_streamPrivate = &stream;
-    static Atomic<uint32_t> pipelineId;
-    auto pipelineName = makeString("mediastream-", pipelineId.exchangeAdd(1));
-
-    loadFull(String("mediastream://") + stream.id(), pipelineName);
+    load(String("mediastream://") + stream.id());
     syncOnClock(false);
 
     m_player->play();
@@ -1543,17 +1535,6 @@ void MediaPlayerPrivateGStreamer::videoChangedCallback(MediaPlayerPrivateGStream
     });
 }
 
-void MediaPlayerPrivateGStreamer::setPipeline(GstElement* pipeline)
-{
-    if (!pipeline) {
-        GST_WARNING("Playbin not found, make sure to install gst-plugins-base");
-        loadingFailed(MediaPlayer::NetworkState::FormatError, MediaPlayer::ReadyState::HaveNothing, true);
-        return;
-    }
-
-    m_pipeline = pipeline;
-}
-
 void MediaPlayerPrivateGStreamer::handleStreamCollectionMessage(GstMessage* message)
 {
     if (m_isLegacyPlaybin)
@@ -2695,7 +2676,7 @@ static void setPlaybackFlags(GstElement* pipeline)
     GST_DEBUG_OBJECT(pipeline, "current pipeline flags %x", flags);
 }
 
-void MediaPlayerPrivateGStreamer::createGSTPlayBin(const URL& url, const String& pipelineName)
+void MediaPlayerPrivateGStreamer::createGSTPlayBin(const URL& url)
 {
     GST_INFO("Creating pipeline for %s player", m_player->isVideoPlayer() ? "video" : "audio");
     const char* playbinName = "playbin";
@@ -2707,25 +2688,25 @@ void MediaPlayerPrivateGStreamer::createGSTPlayBin(const URL& url, const String&
     if ((!isMediaSource() && usePlaybin3 && equal(usePlaybin3, "1")) || url.protocolIs("mediastream"))
         playbinName = "playbin3";
 
-    if (m_pipeline) {
-        if (!g_strcmp0(GST_OBJECT_NAME(gst_element_get_factory(m_pipeline.get())), playbinName)) {
-            GST_INFO_OBJECT(pipeline(), "Already using %s", playbinName);
-            return;
-        }
+    ASSERT(!m_pipeline);
 
-        GST_INFO_OBJECT(pipeline(), "Tearing down as we need to use %s now.", playbinName);
-        changePipelineState(GST_STATE_NULL);
-        m_pipeline = nullptr;
-        m_audioSink = nullptr;
-    }
+    auto elementId = m_player->elementId();
+    if (elementId.isEmpty())
+        elementId = "media-player";
 
-    ASSERT(!m_pipeline);
+    const char* type = isMediaSource() ? "MSE-" : url.protocolIs("mediastream") ? "mediastream-" : "";
 
     m_isLegacyPlaybin = !g_strcmp0(playbinName, "playbin");
 
     static Atomic<uint32_t> pipelineId;
-    setPipeline(gst_element_factory_make(playbinName,
-        (pipelineName.isEmpty() ? makeString("media-player-", pipelineId.exchangeAdd(1)) : pipelineName).utf8().data()));
+
+    m_pipeline = adoptGRef(gst_element_factory_make(playbinName, makeString(type, elementId, '-', pipelineId.exchangeAdd(1)).ascii().data()));
+    if (!m_pipeline) {
+        GST_WARNING("%s not found, make sure to install gst-plugins-base", playbinName);
+        loadingFailed(MediaPlayer::NetworkState::FormatError, MediaPlayer::ReadyState::HaveNothing, true);
+        return;
+    }
+
     setStreamVolumeElement(GST_STREAM_VOLUME(m_pipeline.get()));
 
     GST_INFO_OBJECT(pipeline(), "Using legacy playbin element: %s", boolForPrinting(m_isLegacyPlaybin));

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
@@ -280,7 +280,6 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
 
     void setStreamVolumeElement(GstStreamVolume*);
 
-    void setPipeline(GstElement*);
     GstElement* pipeline() const { return m_pipeline.get(); }
 
     void repaint();
@@ -411,7 +410,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
     virtual void updateStates();
     virtual void asyncStateChangeDone();
 
-    void createGSTPlayBin(const URL&, const String& pipelineName);
+    void createGSTPlayBin(const URL&);
 
     bool loadNextLocation();
     void mediaLocationChanged(GstMessage*);
@@ -443,7 +442,6 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
     static void downloadBufferFileCreatedCallback(MediaPlayerPrivateGStreamer*);
 
     void setPlaybinURL(const URL& urlString);
-    void loadFull(const String& url, const String& pipelineName);
 
     void updateTracks(const GRefPtr<GstStreamCollection>&);
     void videoSinkCapsChanged(GstPad*);