Bug 123025

Summary:

MediaStreamTrack now tracks its own state

Product:

WebKit

Reporter:

Thiago de Barros Lacerda <thiago.lacerda>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric.carlson, glenn, hta, jer.noble, tommyw

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

124288

Attachments:

Description	Flags
Patch	none
Patch	none

Thiago de Barros Lacerda

Reported 2013-10-18 09:27:49 PDT

The spec says that a MediaStreamSource can be shared by different tracks, so a track must have its own state tracking. In the old implementation if a user invoked the stop method, its readyState method was still returning the MediaStreamSource state, which was wrong. This also adds a setEnabled method, which can be used to set the state of a track when a remote peer ends it, for instance.

Attachments
Patch (6.63 KB, patch) 2013-10-18 09:30 PDT, Thiago de Barros Lacerda	no flags	Details Formatted Diff Diff
Patch (5.52 KB, patch) 2013-10-21 13:33 PDT, Thiago de Barros Lacerda	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Thiago de Barros Lacerda

Comment 1 2013-10-18 09:30:51 PDT

Created attachment 214581 [details] Patch

Jer Noble

Comment 2 2013-10-21 12:44:00 PDT

Comment on attachment 214581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214581&action=review > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:80 > + m_state = m_source ? (TrackState) m_source->readyState() : MediaStreamTrack::New; The space between the cast and variable is unnecessary, but I'm more worried about the cast itself. There should only be one instance of this enum, either in MediaStreamTrack, or MediaStreamSource. So, if you reverted the m_readyState -> m_state change, removed the MediaStreamTrack::TrackState enum, and kept the new setState() method (but renamed it setReadyState()), this patch would be a lot simpler. > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:307 > + setState((TrackState) m_source->readyState()); Ditto.

Thiago de Barros Lacerda

Comment 3 2013-10-21 12:56:31 PDT

(In reply to comment #2) > (From update of attachment 214581 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214581&action=review > > > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:80 > > + m_state = m_source ? (TrackState) m_source->readyState() : MediaStreamTrack::New; > > The space between the cast and variable is unnecessary, but I'm more worried about the cast itself. There should only be one instance of this enum, either in MediaStreamTrack, or MediaStreamSource. > > So, if you reverted the m_readyState -> m_state change, removed the MediaStreamTrack::TrackState enum, and kept the new setState() method (but renamed it setReadyState()), this patch would be a lot simpler. > > > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:307 > > + setState((TrackState) m_source->readyState()); > > Ditto. Sounds good.

Thiago de Barros Lacerda

Comment 4 2013-10-21 13:33:48 PDT

Created attachment 214772 [details] Patch

Jer Noble

Comment 5 2013-10-21 13:37:04 PDT

Comment on attachment 214772 [details] Patch Looks good. r=me.

WebKit Commit Bot

Comment 6 2013-10-21 14:12:58 PDT

Comment on attachment 214772 [details] Patch Clearing flags on attachment: 214772 Committed r157733: <http://trac.webkit.org/changeset/157733>

WebKit Commit Bot

Comment 7 2013-10-21 14:13:01 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.