[Webkit-unassigned] [Bug 68464] Update MediaStream to use WebCore platform interfaces

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68464


Adam Bergkvist <adam.bergkvist at ericsson.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #110604|0                           |1
        is obsolete|                            |
 Attachment #110871|                            |review?
               Flag|                            |




--- Comment #5 from Adam Bergkvist <adam.bergkvist at ericsson.com>  2011-10-13 10:31:23 PST ---
Created an attachment (id=110871)
 --> (https://bugs.webkit.org/attachment.cgi?id=110871&action=review)
Updated patch

(In reply to comment #2)
> (From update of attachment 110604 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110604&action=review
> 
> This looks pretty good.  There's still the question of whether |context| should be first in constructors.

I've updated the patch to follow the new context first convention.

> Also, almost all the places in this patch where we're using unsigned, we probably should be using size_t.

Fixed "internal" usage (left unsigned in the DOM API signatures).

> > Source/WebCore/dom/MediaStream.cpp:48
> > +    MediaStreamTrackVector trackVector;
> > +    unsigned numberOfTracks = m_descriptor->numberOfComponents();
> 
> You can call reserveCapacity on the vector if you know ahead of time how many items you're going to append.

Good point. Fixed.

> > Source/WebCore/dom/MediaStreamTrack.cpp:50
> > +    DEFINE_STATIC_LOCAL(AtomicString, kind, (m_streamDescriptor->component(m_trackIndex)->source()->type() == MediaStreamSource::TypeAudio ? "audio" : "video"));
> 
> It's strange to use a static local that's based on member variables...  This doesn't seem right.

True. That was a bad last minute optimization. :) Fixed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.