[MediaStream API] allow a stream source to be shared
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Oct 2013 22:22:21 +0000 (22:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Oct 2013 22:22:21 +0000 (22:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121954

Patch by Thiago de Barros Lacerda <thiago.lacerda@openbossa.org> on 2013-10-24
Reviewed by Eric Carlson.

Now, the MediaStreamSource don't know about the MediaStream that owns it,
since there can be more than one MediaStream that has it as source for some track.
MediaStreamTrack classes now have observers registered, in case there are more than
one MediaStream owning that track

No new tests, no change in functionality.

* Modules/mediastream/MediaStream.cpp:
(WebCore::MediaStream::MediaStream): Adding the MediaStream as an observer for each track it owns.

(WebCore::MediaStream::addTrack): Now adding the MediaStream as an observer the new added track
and adding the source to the MediaStreamDescriptor.

(WebCore::MediaStream::removeTrack): Instead of removing the source right away, we first check if
there isn't any other track using that source, if not we remove the source.

(WebCore::MediaStream::haveTrackWithSource):
(WebCore::MediaStream::addRemoteSource): MediaStreamSource has no information about the MediaStream
that uses it, so now we don't set the stream in the source anymore.

(WebCore::MediaStream::removeRemoteSource): There can be more than on track using the source. So we
get each track that is using the source and then remove it and fire the ended event.

* Modules/mediastream/MediaStream.h:
* Modules/mediastream/MediaStreamTrack.cpp:
(WebCore::MediaStreamTrack::addObserver):
(WebCore::MediaStreamTrack::removeObserver):
(WebCore::MediaStreamTrack::trackDidEnd): Does not get the client from the MediaStreamDescriptor, it now
notify each of its observers that the track ended.

* Modules/mediastream/MediaStreamTrack.h: Adding Observer class.

* platform/mediastream/MediaStreamDescriptor.cpp: Destructor now does nothing. Previously it was setting
each MediaStreamSource's descriptor to null.

(WebCore::MediaStreamDescriptor::removeSource): Not setting the stream in source anymore.

(WebCore::MediaStreamDescriptor::MediaStreamDescriptor): Ditto.

(WebCore::MediaStreamDescriptor::setEnded): Not setting the state of the source to Ended

* platform/mediastream/MediaStreamDescriptor.h:
(WebCore::MediaStreamDescriptor::~MediaStreamDescriptor):
* platform/mediastream/MediaStreamSource.cpp: Removing references to MediaStream object
(WebCore::MediaStreamSource::MediaStreamSource):
(WebCore::MediaStreamSource::reset):
* platform/mediastream/MediaStreamSource.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@157958 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaStream.cpp
Source/WebCore/Modules/mediastream/MediaStream.h
Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp
Source/WebCore/Modules/mediastream/MediaStreamTrack.h
Source/WebCore/platform/mediastream/MediaStreamDescriptor.cpp
Source/WebCore/platform/mediastream/MediaStreamDescriptor.h
Source/WebCore/platform/mediastream/MediaStreamSource.cpp
Source/WebCore/platform/mediastream/MediaStreamSource.h

index 06d0836..a70e1a4 100644 (file)
@@ -1,3 +1,58 @@
+2013-10-24  Thiago de Barros Lacerda  <thiago.lacerda@openbossa.org>
+
+        [MediaStream API] allow a stream source to be shared
+        https://bugs.webkit.org/show_bug.cgi?id=121954
+
+        Reviewed by Eric Carlson.
+
+        Now, the MediaStreamSource don't know about the MediaStream that owns it,
+        since there can be more than one MediaStream that has it as source for some track.
+        MediaStreamTrack classes now have observers registered, in case there are more than
+        one MediaStream owning that track
+
+        No new tests, no change in functionality.
+
+        * Modules/mediastream/MediaStream.cpp:
+        (WebCore::MediaStream::MediaStream): Adding the MediaStream as an observer for each track it owns.
+
+        (WebCore::MediaStream::addTrack): Now adding the MediaStream as an observer the new added track
+        and adding the source to the MediaStreamDescriptor.
+
+        (WebCore::MediaStream::removeTrack): Instead of removing the source right away, we first check if
+        there isn't any other track using that source, if not we remove the source.
+
+        (WebCore::MediaStream::haveTrackWithSource):
+        (WebCore::MediaStream::addRemoteSource): MediaStreamSource has no information about the MediaStream
+        that uses it, so now we don't set the stream in the source anymore.
+
+        (WebCore::MediaStream::removeRemoteSource): There can be more than on track using the source. So we
+        get each track that is using the source and then remove it and fire the ended event.
+
+        * Modules/mediastream/MediaStream.h:
+        * Modules/mediastream/MediaStreamTrack.cpp:
+        (WebCore::MediaStreamTrack::addObserver):
+        (WebCore::MediaStreamTrack::removeObserver):
+        (WebCore::MediaStreamTrack::trackDidEnd): Does not get the client from the MediaStreamDescriptor, it now
+        notify each of its observers that the track ended.
+
+        * Modules/mediastream/MediaStreamTrack.h: Adding Observer class.
+
+        * platform/mediastream/MediaStreamDescriptor.cpp: Destructor now does nothing. Previously it was setting
+        each MediaStreamSource's descriptor to null.
+
+        (WebCore::MediaStreamDescriptor::removeSource): Not setting the stream in source anymore.
+
+        (WebCore::MediaStreamDescriptor::MediaStreamDescriptor): Ditto.
+
+        (WebCore::MediaStreamDescriptor::setEnded): Not setting the state of the source to Ended
+
+        * platform/mediastream/MediaStreamDescriptor.h:
+        (WebCore::MediaStreamDescriptor::~MediaStreamDescriptor):
+        * platform/mediastream/MediaStreamSource.cpp: Removing references to MediaStream object
+        (WebCore::MediaStreamSource::MediaStreamSource):
+        (WebCore::MediaStreamSource::reset):
+        * platform/mediastream/MediaStreamSource.h:
+
 2013-10-24  Daniel Bates  <dabates@apple.com>
 
         Crash in WebCore::NavigationScheduler::startTimer()
index 7e7e938..ca3a4b8 100644 (file)
@@ -118,15 +118,22 @@ MediaStream::MediaStream(ScriptExecutionContext* context, PassRefPtr<MediaStream
     ASSERT(m_descriptor);
     m_descriptor->setClient(this);
 
+    RefPtr<MediaStreamTrack> track;
     size_t numberOfAudioTracks = m_descriptor->numberOfAudioStreams();
     m_audioTracks.reserveCapacity(numberOfAudioTracks);
-    for (size_t i = 0; i < numberOfAudioTracks; i++)
-        m_audioTracks.append(AudioStreamTrack::create(context, m_descriptor->audioStreams(i)));
+    for (size_t i = 0; i < numberOfAudioTracks; i++) {
+        track = AudioStreamTrack::create(context, m_descriptor->audioStreams(i));
+        track->addObserver(this);
+        m_audioTracks.append(track.release());
+    }
 
     size_t numberOfVideoTracks = m_descriptor->numberOfVideoStreams();
     m_videoTracks.reserveCapacity(numberOfVideoTracks);
-    for (size_t i = 0; i < numberOfVideoTracks; i++)
-        m_videoTracks.append(VideoStreamTrack::create(context, m_descriptor->videoStreams(i)));
+    for (size_t i = 0; i < numberOfVideoTracks; i++) {
+        track = VideoStreamTrack::create(context, m_descriptor->videoStreams(i));
+        track->addObserver(this);
+        m_videoTracks.append(track.release());
+    }
 }
 
 MediaStream::~MediaStream()
@@ -185,6 +192,9 @@ void MediaStream::addTrack(PassRefPtr<MediaStreamTrack> prpTrack, ExceptionCode&
         m_videoTracks.append(track);
         break;
     }
+
+    track->addObserver(this);
+    m_descriptor->addSource(track->source());
 }
 
 void MediaStream::removeTrack(PassRefPtr<MediaStreamTrack> prpTrack, ExceptionCode& ec)
@@ -218,12 +228,36 @@ void MediaStream::removeTrack(PassRefPtr<MediaStreamTrack> prpTrack, ExceptionCo
     if (pos == notFound)
         return;
 
-    m_descriptor->removeSource(track->source());
+    // There can be other tracks using the same source in the same MediaStream,
+    // like when MediaStreamTrack::clone() is called, for instance.
+    // Spec says that a source can be shared, so we must assure that there is no
+    // other track using it.
+    if (!haveTrackWithSource(track->source()))
+        m_descriptor->removeSource(track->source());
 
+    track->removeObserver(this);
     if (!m_audioTracks.size() && !m_videoTracks.size())
         setEnded();
 }
 
+bool MediaStream::haveTrackWithSource(PassRefPtr<MediaStreamSource> source)
+{
+    if (source->type() == MediaStreamSource::Audio) {
+        for (MediaStreamTrackVector::iterator iter = m_audioTracks.begin(); iter != m_audioTracks.end(); ++iter) {
+            if ((*iter)->source() == source.get())
+                return true;
+        }
+        return false;
+    }
+
+    for (MediaStreamTrackVector::iterator iter = m_videoTracks.begin(); iter != m_videoTracks.end(); ++iter) {
+        if ((*iter)->source() == source.get())
+            return true;
+    }
+
+    return false;
+}
+
 MediaStreamTrack* MediaStream::getTrackById(String id)
 {
     for (MediaStreamTrackVector::iterator iter = m_audioTracks.begin(); iter != m_audioTracks.end(); ++iter) {
@@ -271,8 +305,6 @@ void MediaStream::addRemoteSource(MediaStreamSource* source)
     if (ended())
         return;
 
-    source->setStream(descriptor());
-
     RefPtr<MediaStreamTrack> track;
     switch (source->type()) {
     case MediaStreamSource::Audio:
@@ -284,6 +316,7 @@ void MediaStream::addRemoteSource(MediaStreamSource* source)
         m_videoTracks.append(track);
         break;
     }
+    track->addObserver(this);
     m_descriptor->addSource(source);
 
     scheduleDispatchEvent(MediaStreamTrackEvent::create(eventNames().addtrackEvent, false, false, track));
@@ -304,21 +337,20 @@ void MediaStream::removeRemoteSource(MediaStreamSource* source)
         break;
     }
 
-    size_t index = notFound;
+    Vector<int> tracksToRemove;
     for (size_t i = 0; i < tracks->size(); ++i) {
-        if ((*tracks)[i]->source() == source) {
-            index = i;
-            break;
-        }
+        if ((*tracks)[i]->source() == source)
+            tracksToRemove.append(i);
     }
-    if (index == notFound)
-        return;
 
     m_descriptor->removeSource(source);
 
-    RefPtr<MediaStreamTrack> track = (*tracks)[index];
-    tracks->remove(index);
-    scheduleDispatchEvent(MediaStreamTrackEvent::create(eventNames().removetrackEvent, false, false, track));
+    for (int i = tracksToRemove.size() - 1; i >= 0; i--) {
+        RefPtr<MediaStreamTrack> track = (*tracks)[tracksToRemove[i]];
+        track->removeObserver(this);
+        tracks->remove(tracksToRemove[i]);
+        scheduleDispatchEvent(MediaStreamTrackEvent::create(eventNames().removetrackEvent, false, false, track.release()));
+    }
 }
 
 void MediaStream::scheduleDispatchEvent(PassRefPtr<Event> event)
index fc43391..2b287b9 100644 (file)
@@ -98,6 +98,8 @@ private:
     virtual void addRemoteSource(MediaStreamSource*) OVERRIDE FINAL;
     virtual void removeRemoteSource(MediaStreamSource*) OVERRIDE FINAL;
 
+    bool haveTrackWithSource(PassRefPtr<MediaStreamSource>);
+
     void scheduleDispatchEvent(PassRefPtr<Event>);
     void scheduledEventTimerFired(Timer<MediaStream>*);
 
index d1bc34b..aa0de5f 100644 (file)
@@ -296,6 +296,18 @@ bool MediaStreamTrack::ended() const
     return m_stopped || m_readyState == MediaStreamSource::Ended;
 }
 
+void MediaStreamTrack::addObserver(MediaStreamTrack::Observer* observer)
+{
+    m_observers.append(observer);
+}
+
+void MediaStreamTrack::removeObserver(MediaStreamTrack::Observer* observer)
+{
+    size_t pos = m_observers.find(observer);
+    if (pos != notFound)
+        m_observers.remove(pos);
+}
+
 void MediaStreamTrack::sourceStateChanged()
 {
     if (m_stopped)
@@ -347,13 +359,8 @@ bool MediaStreamTrack::stopped()
 
 void MediaStreamTrack::trackDidEnd()
 {
-    // FIXME: this is wrong, the track shouldn't have to call the descriptor's client!
-    MediaStreamDescriptorClient* client = m_source ? m_source->stream()->client() : 0;
-    if (!client)
-        return;
-    
-    client->trackDidEnd();
-    setState(MediaStreamSource::Ended);
+    for (Vector<Observer*>::iterator i = m_observers.begin(); i != m_observers.end(); ++i)
+        (*i)->trackDidEnd();
 }
 
 void MediaStreamTrack::stop()
index b50b9d4..d7ea27b 100644 (file)
@@ -49,6 +49,11 @@ class MediaTrackConstraints;
 
 class MediaStreamTrack : public RefCounted<MediaStreamTrack>, public ScriptWrappable, public ActiveDOMObject, public EventTargetWithInlineData, public MediaStreamSource::Observer {
 public:
+    class Observer {
+    public:
+        virtual void trackDidEnd() = 0;
+    };
+
     virtual ~MediaStreamTrack();
 
     virtual const AtomicString& kind() const = 0;
@@ -86,6 +91,9 @@ public:
 
     bool ended() const;
 
+    void addObserver(Observer*);
+    void removeObserver(Observer*);
+
     // EventTarget
     virtual EventTargetInterface eventTargetInterface() const OVERRIDE FINAL { return MediaStreamTrackEventTargetInterfaceType; }
     virtual ScriptExecutionContext* scriptExecutionContext() const OVERRIDE FINAL { return ActiveDOMObject::scriptExecutionContext(); }
@@ -125,6 +133,8 @@ private:
     mutable String m_id;
     Mutex m_mutex;
 
+    Vector<Observer*> m_observers;
+
     bool m_stopped;
     bool m_enabled;
     bool m_muted;
index c2f8d15..dc4b18b 100644 (file)
@@ -48,15 +48,6 @@ PassRefPtr<MediaStreamDescriptor> MediaStreamDescriptor::create(const MediaStrea
     return adoptRef(new MediaStreamDescriptor(createCanonicalUUIDString(), audioSources, videoSources, flag == IsEnded));
 }
 
-MediaStreamDescriptor::~MediaStreamDescriptor()
-{
-    for (size_t i = 0; i < m_audioStreamSources.size(); i++)
-        m_audioStreamSources[i]->setStream(0);
-    
-    for (size_t i = 0; i < m_videoStreamSources.size(); i++)
-        m_videoStreamSources[i]->setStream(0);
-}
-
 void MediaStreamDescriptor::addSource(PassRefPtr<MediaStreamSource> source)
 {
     switch (source->type()) {
@@ -88,8 +79,6 @@ void MediaStreamDescriptor::removeSource(PassRefPtr<MediaStreamSource> source)
         m_videoStreamSources.remove(pos);
         break;
     }
-
-    source->setStream(0);
 }
 
 void MediaStreamDescriptor::addRemoteSource(MediaStreamSource* source)
@@ -114,26 +103,19 @@ MediaStreamDescriptor::MediaStreamDescriptor(const String& id, const MediaStream
     , m_ended(ended)
 {
     ASSERT(m_id.length());
-    for (size_t i = 0; i < audioSources.size(); i++) {
-        audioSources[i]->setStream(this);
+    for (size_t i = 0; i < audioSources.size(); i++)
         m_audioStreamSources.append(audioSources[i]);
-    }
 
-    for (size_t i = 0; i < videoSources.size(); i++) {
-        videoSources[i]->setStream(this);
+    for (size_t i = 0; i < videoSources.size(); i++)
         m_videoStreamSources.append(videoSources[i]);
-    }
 }
 
 void MediaStreamDescriptor::setEnded()
 {
     if (m_client)
         m_client->streamDidEnd();
+
     m_ended = true;
-    for (size_t i = 0; i < m_audioStreamSources.size(); i++)
-        m_audioStreamSources[i]->setReadyState(MediaStreamSource::Ended);
-    for (size_t i = 0; i < m_videoStreamSources.size(); i++)
-        m_videoStreamSources[i]->setReadyState(MediaStreamSource::Ended);
 }
 
 } // namespace WebCore
index 45284ab..938961b 100644 (file)
 #if ENABLE(MEDIA_STREAM)
 
 #include "MediaStreamSource.h"
+#include "MediaStreamTrack.h"
 #include <wtf/RefCounted.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
 
-class MediaStreamDescriptorClient {
+class MediaStreamDescriptorClient : public MediaStreamTrack::Observer {
 public:
     virtual ~MediaStreamDescriptorClient() { }
 
-    virtual void trackDidEnd() = 0;
     virtual void streamDidEnd() = 0;
     virtual void addRemoteSource(MediaStreamSource*) = 0;
     virtual void removeRemoteSource(MediaStreamSource*) = 0;
@@ -56,7 +56,7 @@ public:
 
     static PassRefPtr<MediaStreamDescriptor> create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, EndedAtCreationFlag);
 
-    virtual ~MediaStreamDescriptor();
+    virtual ~MediaStreamDescriptor() { }
 
     MediaStreamDescriptorClient* client() const { return m_client; }
     void setClient(MediaStreamDescriptorClient* client) { m_client = client; }
index 6dbdb97..2d1d17c 100644 (file)
@@ -48,7 +48,6 @@ MediaStreamSource::MediaStreamSource(const String& id, Type type, const String&
     , m_type(type)
     , m_name(name)
     , m_readyState(New)
-    , m_stream(0)
     , m_enabled(true)
     , m_muted(false)
     , m_readonly(false)
@@ -63,7 +62,6 @@ MediaStreamSource::MediaStreamSource(const String& id, Type type, const String&
 void MediaStreamSource::reset()
 {
     m_readyState = New;
-    m_stream = 0;
     m_enabled = true;
     m_muted = false;
     m_readonly = false;
@@ -92,13 +90,6 @@ void MediaStreamSource::removeObserver(MediaStreamSource::Observer* observer)
         m_observers.remove(pos);
 }
 
-void MediaStreamSource::setStream(MediaStreamDescriptor* stream)
-{
-    // FIXME: A source should not need to know about its stream(s). This will be fixed as a part of
-    // https://bugs.webkit.org/show_bug.cgi?id=121954
-    m_stream = stream;
-}
-
 MediaConstraints* MediaStreamSource::constraints() const
 {
     // FIXME: While this returns 
index 2056419..1ed3fdc 100644 (file)
@@ -100,9 +100,6 @@ public:
 
     void stop();
 
-    MediaStreamDescriptor* stream() const { return m_stream; }
-    void setStream(MediaStreamDescriptor*);
-    
 protected:
     MediaStreamSource(const String& id, Type, const String& name);
 
@@ -113,7 +110,6 @@ private:
     ReadyState m_readyState;
     Vector<Observer*> m_observers;
     RefPtr<MediaConstraints> m_constraints;
-    MediaStreamDescriptor* m_stream;
     MediaStreamSourceStates m_states;
 
     bool m_enabled;