[GStreamer][WebRTC] Audio is not played from an audio element when th…

…e srcObject object has unstarted video tracks https://bugs.webkit.org/show_bug.cgi?id=209163 Patch by Philippe Normand <pnormand@igalia.com> on 2021-04-06 Reviewed by Xabier Rodriguez-Calvar. Source/WebCore: Ensure no MediaStream (active) video tracks can be added in a pipeline representing an <audio> element. * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::sourceSetup): (WebCore::MediaPlayerPrivateGStreamer::hasFirstSampleReachedSink const): (WebCore::MediaPlayerPrivateGStreamer::videoSinkCapsChanged): (WebCore::MediaPlayerPrivateGStreamer::playbackPosition const): (WebCore::MediaPlayerPrivateGStreamer::updateTracks): * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: * platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp: (webkitMediaStreamSrcPostStreamCollection): (webkitMediaStreamSrcSetStream): * platform/mediastream/gstreamer/GStreamerMediaStreamSource.h: LayoutTests: * platform/glib/TestExpectations: Unflag now-passing test. Canonical link: https://commits.webkit.org/236171@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275514 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 6, 2021 · adca9bb7ad481b3438890390c6b457f72a3b90ee · adca9bb
1 parent 6ab97f2
commit adca9bb7ad481b3438890390c6b457f72a3b90ee
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 24 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2021-04-06  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer][WebRTC] Audio is not played from an audio element when the srcObject object has unstarted video tracks
+        https://bugs.webkit.org/show_bug.cgi?id=209163
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * platform/glib/TestExpectations: Unflag now-passing test.
+
 2021-04-06  Keith Miller  <keith_miller@apple.com>
 
         CloneDeserializer should use ArrayBuffer::tryCreate

diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -1070,8 +1070,6 @@ webkit.org/b/206464 webkit.org/b/218787 webrtc/peerconnection-page-cache.html [
 
 webkit.org/b/208125 webrtc/peerconnection-new-candidate-page-cache.html [ Failure Timeout ]
 
-webkit.org/b/209163 webrtc/audio-video-element-playing.html [ Crash Failure Pass ]
-
 webkit.org/b/216538 webrtc/captureCanvas-webrtc-software-h264-baseline.html [ Slow Failure ]
 
 webkit.org/b/216763 webrtc/captureCanvas-webrtc-software-h264-high.html [ Crash Failure ]

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2021-04-06  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer][WebRTC] Audio is not played from an audio element when the srcObject object has unstarted video tracks
+        https://bugs.webkit.org/show_bug.cgi?id=209163
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Ensure no MediaStream (active) video tracks can be added in a pipeline representing an <audio> element.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
+        (WebCore::MediaPlayerPrivateGStreamer::sourceSetup):
+        (WebCore::MediaPlayerPrivateGStreamer::hasFirstSampleReachedSink const):
+        (WebCore::MediaPlayerPrivateGStreamer::videoSinkCapsChanged):
+        (WebCore::MediaPlayerPrivateGStreamer::playbackPosition const):
+        (WebCore::MediaPlayerPrivateGStreamer::updateTracks):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
+        * platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
+        (webkitMediaStreamSrcPostStreamCollection):
+        (webkitMediaStreamSrcSetStream):
+        * platform/mediastream/gstreamer/GStreamerMediaStreamSource.h:
+
 2021-04-06  Philippe Normand  <pnormand@igalia.com>
 
         [GTK][GStreamer] Web Audio - Media element source - Audio is cracking.

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
@@ -872,7 +872,7 @@ void MediaPlayerPrivateGStreamer::sourceSetup(GstElement* sourceElement)
     } else if (WEBKIT_IS_MEDIA_STREAM_SRC(sourceElement)) {
         auto stream = m_streamPrivate.get();
         ASSERT(stream);
-        webkitMediaStreamSrcSetStream(WEBKIT_MEDIA_STREAM_SRC(sourceElement), stream);
+        webkitMediaStreamSrcSetStream(WEBKIT_MEDIA_STREAM_SRC(sourceElement), stream, m_player->isVideoPlayer());
 #endif
     }
 }
@@ -1033,6 +1033,11 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfVideo()
 
     m_player->mediaEngineUpdated();
 }
+bool MediaPlayerPrivateGStreamer::hasFirstVideoSampleReachedSink() const
+{
+    auto sampleLocker = holdLock(m_sampleMutex);
+    return !!m_sample;
+}
 
 void MediaPlayerPrivateGStreamer::videoSinkCapsChanged(GstPad* videoSinkPad)
 {
@@ -1045,14 +1050,7 @@ void MediaPlayerPrivateGStreamer::videoSinkCapsChanged(GstPad* videoSinkPad)
     ASSERT(!isMainThread());
     GST_DEBUG_OBJECT(videoSinkPad, "Received new caps: %" GST_PTR_FORMAT, caps.get());
 
-    bool hasFirstSampleReachedSink;
-    // This actually lacks contention since both notify::caps and triggerRepaint() are both run in the same streaming thread.
-    {
-        auto sampleLocker = holdLock(m_sampleMutex);
-        hasFirstSampleReachedSink = !!m_sample;
-    }
-
-    if (!hasFirstSampleReachedSink) {
+    if (!hasFirstVideoSampleReachedSink()) {
         // We want to wait for the sink to receive the first buffer before emitting dimensions, since only by then we
         // are guaranteed that any potential tag event with a rotation has been handled.
         GST_DEBUG_OBJECT(videoSinkPad, "Ignoring notify::caps until the first buffer reaches the sink.");
@@ -1341,6 +1339,12 @@ GstElement* MediaPlayerPrivateGStreamer::audioSink() const
 MediaTime MediaPlayerPrivateGStreamer::playbackPosition() const
 {
     GST_TRACE_OBJECT(pipeline(), "isEndReached: %s, seeking: %s, seekTime: %s", boolForPrinting(m_isEndReached), boolForPrinting(m_isSeeking), m_seekTime.toString().utf8().data());
+
+#if ENABLE(MEDIA_STREAM)
+    if (m_streamPrivate && m_player->isVideoPlayer() && !hasFirstVideoSampleReachedSink())
+        return MediaTime::zeroTime();
+#endif
+
     if (m_isSeeking)
         return m_seekTime;
     if (m_isEndReached)
@@ -1484,7 +1488,6 @@ void MediaPlayerPrivateGStreamer::updateTracks(const GRefPtr<GstStreamCollection
     unsigned textTrackIndex = 0;
 
 #define CREATE_TRACK(type, Type) G_STMT_START {                         \
-        m_has##Type = true;                                             \
         if (!useMediaSource) {                                          \
             RefPtr<Type##TrackPrivateGStreamer> track = Type##TrackPrivateGStreamer::create(makeWeakPtr(*this), type##TrackIndex, stream); \
             auto trackId = track->id();                                 \
@@ -1508,7 +1511,7 @@ void MediaPlayerPrivateGStreamer::updateTracks(const GRefPtr<GstStreamCollection
 
         if (type & GST_STREAM_TYPE_AUDIO)
             CREATE_TRACK(audio, Audio);
-        else if (type & GST_STREAM_TYPE_VIDEO)
+        else if (type & GST_STREAM_TYPE_VIDEO && m_player->isVideoPlayer())
             CREATE_TRACK(video, Video);
         else if (type & GST_STREAM_TYPE_TEXT && !useMediaSource) {
             auto track = InbandTextTrackPrivateGStreamer::create(textTrackIndex++, stream);
@@ -1518,10 +1521,13 @@ void MediaPlayerPrivateGStreamer::updateTracks(const GRefPtr<GstStreamCollection
             GST_WARNING("Unknown track type found for stream %s", streamId.utf8().data());
     }
 
+    m_hasAudio = !m_audioTracks.isEmpty();
+    m_hasVideo = !m_videoTracks.isEmpty();
+
     if (oldHasVideo != m_hasVideo || oldHasAudio != m_hasAudio)
         m_player->characteristicChanged();
 
-    if (m_hasVideo)
+    if (!oldHasVideo && m_hasVideo)
         m_player->sizeChanged();
 
     m_player->mediaEngineUpdated();

diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
@@ -448,6 +448,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
     void updateTracks(const GRefPtr<GstStreamCollection>&);
     void videoSinkCapsChanged(GstPad*);
     void updateVideoSizeAndOrientationFromCaps(const GstCaps*);
+    bool hasFirstVideoSampleReachedSink() const;
 
 #if ENABLE(ENCRYPTED_MEDIA)
     bool isCDMAttached() const { return m_cdmInstance; }

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp b/Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp
@@ -574,15 +574,18 @@ static void webkitMediaStreamSrcPostStreamCollection(WebKitMediaStreamSrc* self)
     auto* priv = self->priv;
 
     GST_OBJECT_LOCK(self);
-    String upstreamId;
-    if (priv->stream)
-        upstreamId = priv->stream->id();
-    else
-        upstreamId = createCanonicalUUIDString();
+    if (priv->stream && (!priv->stream->active() || !priv->stream->hasTracks())) {
+        GST_OBJECT_UNLOCK(self);
+        return;
+    }
 
+    auto upstreamId = priv->stream ? priv->stream->id() : createCanonicalUUIDString();
     priv->streamCollection = adoptGRef(gst_stream_collection_new(upstreamId.ascii().data()));
-    for (auto& track : priv->tracks)
+    for (auto& track : priv->tracks) {
+        if (!track->isActive())
+            continue;
         gst_stream_collection_add_stream(priv->streamCollection.get(), webkitMediaStreamNew(track.get()));
+    }
 
     GST_OBJECT_UNLOCK(self);
 
@@ -615,7 +618,7 @@ static void webkitMediaStreamSrcRemoveTrackByType(WebKitMediaStreamSrc* self, Re
         GST_INFO_OBJECT(self, "Unsupported track type: %d", static_cast<int>(trackType));
 }
 
-void webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc* self, MediaStreamPrivate* stream)
+void webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc* self, MediaStreamPrivate* stream, bool isVideoPlayer)
 {
     ASSERT(WEBKIT_IS_MEDIA_STREAM_SRC(self));
     ASSERT(!self->priv->stream);
@@ -624,9 +627,11 @@ void webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc* self, MediaStreamPrivat
     self->priv->stream->addObserver(*self->priv->mediaStreamObserver.get());
     auto tracks = stream->tracks();
     bool onlyTrack = tracks.size() == 1;
-    for (auto& track : tracks)
+    for (auto& track : tracks) {
+        if (!isVideoPlayer && track->type() == RealtimeMediaSource::Type::Video)
+            continue;
         webkitMediaStreamSrcAddTrack(self, track.get(), onlyTrack);
-
+    }
     webkitMediaStreamSrcPostStreamCollection(self);
     gst_element_no_more_pads(GST_ELEMENT_CAST(self));
 }

diff --git a/Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.h b/Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.h
@@ -54,7 +54,7 @@ struct _WebKitMediaStreamSrcClass {
     GstBinClass parentClass;
 };
 
-void webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc*, WebCore::MediaStreamPrivate*);
+void webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc*, WebCore::MediaStreamPrivate*, bool isVideoPlayer);
 void webkitMediaStreamSrcAddTrack(WebKitMediaStreamSrc*, WebCore::MediaStreamTrackPrivate*, bool onlyTrack);
 GstElement* webkitMediaStreamSrcNew();