Title: [158337] trunk/Source/WebCore
Revision
158337
Author
commit-qu...@webkit.org
Date
2013-10-30 18:15:20 -0700 (Wed, 30 Oct 2013)

Log Message

Simplifying MediaStream and MediStreamDescriptor creation
https://bugs.webkit.org/show_bug.cgi?id=123443

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

The internal process of creating a MediaStream and MediaStreamDescriptor was quite confusing and spread.
We can take advantage of the platform implementation of MediaStreamTrack (aka MediaStreamTrackPrivate)
and simplify the whole process.
A new constructor that receives vectors of MediaStreamTrackPrivate objects were added, then the check
if a source already exists or if the tracks are all ended are now made in MediaStreamDescriptor.

No new tests needed.

* Modules/mediastream/MediaStream.cpp:
(WebCore::MediaStream::create): Removed unnecessary variables in one create method and using new
MediaStreamDescriptor::create method that receives vector of MediaStreamTrackPrivate objects as parameter.

* Modules/webaudio/MediaStreamAudioDestinationNode.cpp:
(WebCore::MediaStreamAudioDestinationNode::MediaStreamAudioDestinationNode): Removed passing flag to
MediaStreamDescriptor create.

* platform/mediastream/MediaStreamDescriptor.cpp:
(WebCore::MediaStreamDescriptor::create): Removed EndedAtCreationFlag parameter, because this is being
handled inside constructor by analyzing the tracks or sources passed.
(WebCore::MediaStreamDescriptor::MediaStreamDescriptor): Adding new constructor that receives vector of
MediaStreamTrackPrivate as parameter.

(WebCore::MediaStreamDescriptor::addTrack): Changed to store the track's source in the object.

(WebCore::MediaStreamDescriptor::removeTrack):
* platform/mediastream/MediaStreamDescriptor.h:
(WebCore::MediaStreamDescriptor::numberOfAudioTracks):
(WebCore::MediaStreamDescriptor::audioTracks):
(WebCore::MediaStreamDescriptor::numberOfVideoTracks):
(WebCore::MediaStreamDescriptor::videoTracks):
* platform/mock/MockMediaStreamCenter.cpp:
(WebCore::MockMediaStreamCenter::createMediaStream): Removing flag that is being passed to
MediaStreamDescriptor's create method.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (158336 => 158337)


--- trunk/Source/WebCore/ChangeLog	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/ChangeLog	2013-10-31 01:15:20 UTC (rev 158337)
@@ -1,3 +1,44 @@
+2013-10-30  Thiago de Barros Lacerda  <thiago.lace...@openbossa.org>
+
+        Simplifying MediaStream and MediStreamDescriptor creation
+        https://bugs.webkit.org/show_bug.cgi?id=123443
+
+        Reviewed by Eric Carlson.
+
+        The internal process of creating a MediaStream and MediaStreamDescriptor was quite confusing and spread.
+        We can take advantage of the platform implementation of MediaStreamTrack (aka MediaStreamTrackPrivate)
+        and simplify the whole process.
+        A new constructor that receives vectors of MediaStreamTrackPrivate objects were added, then the check
+        if a source already exists or if the tracks are all ended are now made in MediaStreamDescriptor.
+
+        No new tests needed.
+
+        * Modules/mediastream/MediaStream.cpp:
+        (WebCore::MediaStream::create): Removed unnecessary variables in one create method and using new
+        MediaStreamDescriptor::create method that receives vector of MediaStreamTrackPrivate objects as parameter.
+
+        * Modules/webaudio/MediaStreamAudioDestinationNode.cpp:
+        (WebCore::MediaStreamAudioDestinationNode::MediaStreamAudioDestinationNode): Removed passing flag to
+        MediaStreamDescriptor create.
+
+        * platform/mediastream/MediaStreamDescriptor.cpp:
+        (WebCore::MediaStreamDescriptor::create): Removed EndedAtCreationFlag parameter, because this is being
+        handled inside constructor by analyzing the tracks or sources passed.
+        (WebCore::MediaStreamDescriptor::MediaStreamDescriptor): Adding new constructor that receives vector of
+        MediaStreamTrackPrivate as parameter.
+
+        (WebCore::MediaStreamDescriptor::addTrack): Changed to store the track's source in the object.
+
+        (WebCore::MediaStreamDescriptor::removeTrack):
+        * platform/mediastream/MediaStreamDescriptor.h:
+        (WebCore::MediaStreamDescriptor::numberOfAudioTracks):
+        (WebCore::MediaStreamDescriptor::audioTracks):
+        (WebCore::MediaStreamDescriptor::numberOfVideoTracks):
+        (WebCore::MediaStreamDescriptor::videoTracks):
+        * platform/mock/MockMediaStreamCenter.cpp:
+        (WebCore::MockMediaStreamCenter::createMediaStream): Removing flag that is being passed to
+        MediaStreamDescriptor's create method.
+
 2013-10-30  Brent Fulgham  <bfulg...@apple.com>
 
         [Win] Legible Output callbacks should happen on notification queue

Modified: trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp (158336 => 158337)


--- trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp	2013-10-31 01:15:20 UTC (rev 158337)
@@ -41,68 +41,40 @@
 
 namespace WebCore {
 
-static bool containsSource(MediaStreamSourceVector& sourceVector, MediaStreamSource* source)
-{
-    for (size_t i = 0; i < sourceVector.size(); ++i) {
-        if (source->id() == sourceVector[i]->id())
-            return true;
-    }
-    return false;
-}
-
-static void processTrack(MediaStreamTrack* track, MediaStreamSourceVector& sourceVector)
-{
-    if (track->ended())
-        return;
-
-    MediaStreamSource* source = track->source();
-    if (!containsSource(sourceVector, source))
-        sourceVector.append(source);
-}
-
-static PassRefPtr<MediaStream> createFromSourceVectors(ScriptExecutionContext* context, const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, bool ended)
-{
-    MediaStreamDescriptor::EndedAtCreationFlag flag = ended ? MediaStreamDescriptor::IsEnded : MediaStreamDescriptor::IsNotEnded;
-    RefPtr<MediaStreamDescriptor> descriptor = MediaStreamDescriptor::create(audioSources, videoSources, flag);
-    return MediaStream::create(context, descriptor.release());
-}
-
 PassRefPtr<MediaStream> MediaStream::create(ScriptExecutionContext* context)
 {
-    MediaStreamSourceVector audioSources;
-    MediaStreamSourceVector videoSources;
-
-    return createFromSourceVectors(context, audioSources, videoSources, false);
+    return MediaStream::create(context, MediaStreamDescriptor::create(MediaStreamSourceVector(), MediaStreamSourceVector()));
 }
 
 PassRefPtr<MediaStream> MediaStream::create(ScriptExecutionContext* context, PassRefPtr<MediaStream> stream)
 {
     ASSERT(stream);
 
-    MediaStreamSourceVector audioSources;
-    MediaStreamSourceVector videoSources;
+    Vector<RefPtr<MediaStreamTrackPrivate>> audioTracks;
+    Vector<RefPtr<MediaStreamTrackPrivate>> videoTracks;
 
     for (size_t i = 0; i < stream->m_audioTracks.size(); ++i)
-        processTrack(stream->m_audioTracks[i].get(), audioSources);
+        audioTracks.append(&stream->m_audioTracks[i]->privateTrack());
 
     for (size_t i = 0; i < stream->m_videoTracks.size(); ++i)
-        processTrack(stream->m_videoTracks[i].get(), videoSources);
+        videoTracks.append(&stream->m_videoTracks[i]->privateTrack());
 
-    return createFromSourceVectors(context, audioSources, videoSources, stream->ended());
+    return MediaStream::create(context, MediaStreamDescriptor::create(audioTracks, videoTracks));
 }
 
 PassRefPtr<MediaStream> MediaStream::create(ScriptExecutionContext* context, const MediaStreamTrackVector& tracks)
 {
-    MediaStreamSourceVector audioSources;
-    MediaStreamSourceVector videoSources;
+    Vector<RefPtr<MediaStreamTrackPrivate>> audioTracks;
+    Vector<RefPtr<MediaStreamTrackPrivate>> videoTracks;
 
-    bool allEnded = true;
     for (size_t i = 0; i < tracks.size(); ++i) {
-        allEnded &= tracks[i]->ended();
-        processTrack(tracks[i].get(), tracks[i]->kind() == "audio" ? audioSources : videoSources);
+        if (tracks[i]->kind() == "audio")
+            audioTracks.append(&tracks[i]->privateTrack());
+        else
+            videoTracks.append(&tracks[i]->privateTrack());
     }
 
-    return createFromSourceVectors(context, audioSources, videoSources, allEnded);
+    return MediaStream::create(context, MediaStreamDescriptor::create(audioTracks, videoTracks));
 }
 
 PassRefPtr<MediaStream> MediaStream::create(ScriptExecutionContext* context, PassRefPtr<MediaStreamDescriptor> streamDescriptor)

Modified: trunk/Source/WebCore/Modules/mediastream/MediaStreamTrack.h (158336 => 158337)


--- trunk/Source/WebCore/Modules/mediastream/MediaStreamTrack.h	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/Modules/mediastream/MediaStreamTrack.h	2013-10-31 01:15:20 UTC (rev 158337)
@@ -90,6 +90,7 @@
     DEFINE_ATTRIBUTE_EVENT_LISTENER(overconstrained);
 
     MediaStreamSource* source() const { return m_privateTrack->source(); }
+    MediaStreamTrackPrivate& privateTrack() { return m_privateTrack.get(); }
 
     bool ended() const;
 
@@ -107,7 +108,6 @@
     explicit MediaStreamTrack(MediaStreamTrack*);
     MediaStreamTrack(ScriptExecutionContext*, MediaStreamTrackPrivate&, const Dictionary*);
 
-    MediaStreamTrackPrivate& privateTrack() { return m_privateTrack.get(); }
     void setSource(PassRefPtr<MediaStreamSource>);
 
 private:

Modified: trunk/Source/WebCore/Modules/webaudio/MediaStreamAudioDestinationNode.cpp (158336 => 158337)


--- trunk/Source/WebCore/Modules/webaudio/MediaStreamAudioDestinationNode.cpp	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/Modules/webaudio/MediaStreamAudioDestinationNode.cpp	2013-10-31 01:15:20 UTC (rev 158337)
@@ -51,7 +51,7 @@
     m_source = MediaStreamAudioSource::create();
     MediaStreamSourceVector audioSources;
     audioSources.append(m_source);
-    m_stream = MediaStream::create(context->scriptExecutionContext(), MediaStreamDescriptor::create(audioSources, MediaStreamSourceVector(), MediaStreamDescriptor::IsNotEnded));
+    m_stream = MediaStream::create(context->scriptExecutionContext(), MediaStreamDescriptor::create(audioSources, MediaStreamSourceVector()));
 
     m_source->setAudioFormat(numberOfChannels, context->sampleRate());
 

Modified: trunk/Source/WebCore/platform/mediastream/MediaStreamDescriptor.cpp (158336 => 158337)


--- trunk/Source/WebCore/platform/mediastream/MediaStreamDescriptor.cpp	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/platform/mediastream/MediaStreamDescriptor.cpp	2013-10-31 01:15:20 UTC (rev 158337)
@@ -36,19 +36,22 @@
 #include "MediaStreamDescriptor.h"
 
 #include "MediaStreamCenter.h"
-#include "MediaStreamSource.h"
-#include "MediaStreamTrackPrivate.h"
 #include "UUID.h"
 #include <wtf/RefCounted.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
 
-PassRefPtr<MediaStreamDescriptor> MediaStreamDescriptor::create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, EndedAtCreationFlag flag)
+PassRefPtr<MediaStreamDescriptor> MediaStreamDescriptor::create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources)
 {
-    return adoptRef(new MediaStreamDescriptor(createCanonicalUUIDString(), audioSources, videoSources, flag == IsEnded));
+    return adoptRef(new MediaStreamDescriptor(createCanonicalUUIDString(), audioSources, videoSources));
 }
 
+PassRefPtr<MediaStreamDescriptor> MediaStreamDescriptor::create(const Vector<RefPtr<MediaStreamTrackPrivate>>& audioPrivateTracks, const Vector<RefPtr<MediaStreamTrackPrivate>>& videoPrivateTracks)
+{
+    return adoptRef(new MediaStreamDescriptor(createCanonicalUUIDString(), audioPrivateTracks, videoPrivateTracks));
+}
+
 void MediaStreamDescriptor::addSource(PassRefPtr<MediaStreamSource> source)
 {
     switch (source->type()) {
@@ -104,25 +107,46 @@
         removeSource(source);
 }
 
-MediaStreamDescriptor::MediaStreamDescriptor(const String& id, const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, bool ended)
+MediaStreamDescriptor::MediaStreamDescriptor(const String& id, const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources)
     : m_client(0)
     , m_id(id)
-    , m_ended(ended)
+    , m_ended(false)
 {
     ASSERT(m_id.length());
-    for (size_t i = 0; i < audioSources.size(); i++) {
-        RefPtr<MediaStreamSource> source = audioSources[i];
-        m_audioStreamSources.append(source);
-        m_audioTrackDescriptors.append(MediaStreamTrackPrivate::create(source));
-    }
+    for (size_t i = 0; i < audioSources.size(); i++)
+        addTrack(MediaStreamTrackPrivate::create(audioSources[i]));
 
-    for (size_t i = 0; i < videoSources.size(); i++) {
-        RefPtr<MediaStreamSource> source = videoSources[i];
-        m_videoStreamSources.append(source);
-        m_videoTrackDescriptors.append(MediaStreamTrackPrivate::create(source));
-    }
+    for (size_t i = 0; i < videoSources.size(); i++)
+        addTrack(MediaStreamTrackPrivate::create(videoSources[i]));
+
+    unsigned providedSourcesSize = audioSources.size() + videoSources.size();
+    unsigned tracksSize = m_audioPrivateTracks.size() + m_videoPrivateTracks.size();
+    // If sources were provided and no track was added to the MediaStreamDescriptor's tracks, this means
+    // that the tracks were all ended
+    if (providedSourcesSize > 0 && !tracksSize)
+        m_ended = true;
 }
 
+MediaStreamDescriptor::MediaStreamDescriptor(const String& id, const Vector<RefPtr<MediaStreamTrackPrivate>>& audioPrivateTracks, const Vector<RefPtr<MediaStreamTrackPrivate>>& videoPrivateTracks)
+    : m_client(0)
+    , m_id(id)
+    , m_ended(false)
+{
+    ASSERT(m_id.length());
+    for (size_t i = 0; i < audioPrivateTracks.size(); i++)
+        addTrack(audioPrivateTracks[i]);
+
+    for (size_t i = 0; i < videoPrivateTracks.size(); i++)
+        addTrack(videoPrivateTracks[i]);
+
+    unsigned providedTracksSize = audioPrivateTracks.size() + videoPrivateTracks.size();
+    unsigned tracksSize = m_audioPrivateTracks.size() + m_videoPrivateTracks.size();
+    // If tracks were provided and no one was added to the MediaStreamDescriptor's tracks, this means
+    // that the tracks were all ended
+    if (providedTracksSize > 0 && !tracksSize)
+        m_ended = true;
+}
+
 void MediaStreamDescriptor::setEnded()
 {
     if (m_client)
@@ -133,18 +157,23 @@
 
 void MediaStreamDescriptor::addTrack(PassRefPtr<MediaStreamTrackPrivate> track)
 {
-    Vector<RefPtr<MediaStreamTrackPrivate>>& tracks = track->type() == MediaStreamSource::Audio ? m_audioTrackDescriptors : m_videoTrackDescriptors;
+    if (track->ended())
+        return;
 
+    Vector<RefPtr<MediaStreamTrackPrivate>>& tracks = track->type() == MediaStreamSource::Audio ? m_audioPrivateTracks : m_videoPrivateTracks;
+
     size_t pos = tracks.find(track);
     if (pos != notFound)
         return;
 
     tracks.append(track);
+    if (track->source())
+        addSource(track->source());
 }
 
 void MediaStreamDescriptor::removeTrack(PassRefPtr<MediaStreamTrackPrivate> track)
 {
-    Vector<RefPtr<MediaStreamTrackPrivate>>& tracks = track->type() == MediaStreamSource::Audio ? m_audioTrackDescriptors : m_videoTrackDescriptors;
+    Vector<RefPtr<MediaStreamTrackPrivate>>& tracks = track->type() == MediaStreamSource::Audio ? m_audioPrivateTracks : m_videoPrivateTracks;
 
     size_t pos = tracks.find(track);
     if (pos == notFound)

Modified: trunk/Source/WebCore/platform/mediastream/MediaStreamDescriptor.h (158336 => 158337)


--- trunk/Source/WebCore/platform/mediastream/MediaStreamDescriptor.h	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/platform/mediastream/MediaStreamDescriptor.h	2013-10-31 01:15:20 UTC (rev 158337)
@@ -36,6 +36,7 @@
 
 #include "MediaStreamSource.h"
 #include "MediaStreamTrack.h"
+#include "MediaStreamTrackPrivate.h"
 #include <wtf/RefCounted.h>
 #include <wtf/Vector.h>
 
@@ -54,10 +55,9 @@
 
 class MediaStreamDescriptor : public RefCounted<MediaStreamDescriptor> {
 public:
-    enum EndedAtCreationFlag { IsNotEnded, IsEnded };
+    static PassRefPtr<MediaStreamDescriptor> create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources);
+    static PassRefPtr<MediaStreamDescriptor> create(const Vector<RefPtr<MediaStreamTrackPrivate>>& audioPrivateTracks, const Vector<RefPtr<MediaStreamTrackPrivate>>& videoPrivateTracks);
 
-    static PassRefPtr<MediaStreamDescriptor> create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, EndedAtCreationFlag);
-
     virtual ~MediaStreamDescriptor() { }
 
     MediaStreamDescriptorClient* client() const { return m_client; }
@@ -71,11 +71,11 @@
     unsigned numberOfVideoSources() const { return m_videoStreamSources.size(); }
     MediaStreamSource* videoSources(unsigned index) const { return m_videoStreamSources[index].get(); }
 
-    unsigned numberOfAudioTracks() const { return m_audioTrackDescriptors.size(); }
-    MediaStreamTrackPrivate* audioTracks(unsigned index) const { return m_audioTrackDescriptors[index].get(); }
+    unsigned numberOfAudioTracks() const { return m_audioPrivateTracks.size(); }
+    MediaStreamTrackPrivate* audioTracks(unsigned index) const { return m_audioPrivateTracks[index].get(); }
 
-    unsigned numberOfVideoTracks() const { return m_videoTrackDescriptors.size(); }
-    MediaStreamTrackPrivate* videoTracks(unsigned index) const { return m_videoTrackDescriptors[index].get(); }
+    unsigned numberOfVideoTracks() const { return m_videoPrivateTracks.size(); }
+    MediaStreamTrackPrivate* videoTracks(unsigned index) const { return m_videoPrivateTracks[index].get(); }
 
     void addSource(PassRefPtr<MediaStreamSource>);
     void removeSource(PassRefPtr<MediaStreamSource>);
@@ -90,15 +90,16 @@
     void removeTrack(PassRefPtr<MediaStreamTrackPrivate>);
 
 private:
-    MediaStreamDescriptor(const String& id, const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, bool ended);
+    MediaStreamDescriptor(const String& id, const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources);
+    MediaStreamDescriptor(const String& id, const Vector<RefPtr<MediaStreamTrackPrivate>>& audioPrivateTracks, const Vector<RefPtr<MediaStreamTrackPrivate>>& videoPrivateTracks);
 
     MediaStreamDescriptorClient* m_client;
     String m_id;
-    Vector<RefPtr<MediaStreamSource>> m_audioStreamSources;
-    Vector<RefPtr<MediaStreamSource>> m_videoStreamSources;
+    MediaStreamSourceVector m_audioStreamSources;
+    MediaStreamSourceVector m_videoStreamSources;
 
-    Vector<RefPtr<MediaStreamTrackPrivate>> m_audioTrackDescriptors;
-    Vector<RefPtr<MediaStreamTrackPrivate>> m_videoTrackDescriptors;
+    Vector<RefPtr<MediaStreamTrackPrivate>> m_audioPrivateTracks;
+    Vector<RefPtr<MediaStreamTrackPrivate>> m_videoPrivateTracks;
     bool m_ended;
 };
 

Modified: trunk/Source/WebCore/platform/mock/MockMediaStreamCenter.cpp (158336 => 158337)


--- trunk/Source/WebCore/platform/mock/MockMediaStreamCenter.cpp	2013-10-31 01:07:31 UTC (rev 158336)
+++ trunk/Source/WebCore/platform/mock/MockMediaStreamCenter.cpp	2013-10-31 01:15:20 UTC (rev 158337)
@@ -229,7 +229,7 @@
         videoSources.append(videoSource.release());
     }
     
-    client->didCreateStream(MediaStreamDescriptor::create(audioSources, videoSources, MediaStreamDescriptor::IsNotEnded));
+    client->didCreateStream(MediaStreamDescriptor::create(audioSources, videoSources));
 }
 
 bool MockMediaStreamCenter::getMediaStreamTrackSources(PassRefPtr<MediaStreamTrackSourcesRequestClient> prpClient)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to