WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
111293
MediaStream.ended must return true when it is created with ended tracks.
https://bugs.webkit.org/show_bug.cgi?id=111293
Summary
MediaStream.ended must return true when it is created with ended tracks.
Li Yin
Reported
2013-03-04 01:11:58 PST
Spec:
http://dev.w3.org/2011/webrtc/editor/getusermedia.html#MediaStream-ended
When a MediaStream object is created, its ended attribute must be set to false, unless it is being created using the MediaStream() constructor whose arguments are lists of MediaStreamTrack objects that are all ended, in which case the MediaStream object must be created with its ended attribute set to true.
Attachments
Patch
(8.23 KB, patch)
2013-03-04 01:37 PST
,
Li Yin
no flags
Details
Formatted Diff
Diff
Patch
(9.12 KB, patch)
2013-03-04 04:21 PST
,
Li Yin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Li Yin
Comment 1
2013-03-04 01:37:43 PST
Created
attachment 191175
[details]
Patch
Tommy Widenflycht
Comment 2
2013-03-04 01:59:43 PST
Comment on
attachment 191175
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=191175&action=review
Unofficial R+ from me.
> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:114 > + m_ended = true;
To the actual reviewer: The sources are filtered in Source/WebCore/Modules/mediastream/MediaStream.cpp in the function processTrack so when the execution comes to this stage all ended tracks have been ignored.
> LayoutTests/fast/mediastream/MediaStreamConstructor.html:88 > + var ended = arguments[3]?arguments[3]:false;
Nit: I would prefer ended to be added as a proper argument. Also spaces around the operators is the norm.
Kentaro Hara
Comment 3
2013-03-04 02:29:36 PST
Comment on
attachment 191175
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=191175&action=review
>> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:114 >> + m_ended = true; > > To the actual reviewer: > The sources are filtered in Source/WebCore/Modules/mediastream/MediaStream.cpp in the function processTrack so when the execution comes to this stage all ended tracks have been ignored.
Can we add some ASSERT() about this?
>> LayoutTests/fast/mediastream/MediaStreamConstructor.html:88 >> + var ended = arguments[3]?arguments[3]:false; > > Nit: I would prefer ended to be added as a proper argument. Also spaces around the operators is the norm.
Agreed.
Li Yin
Comment 4
2013-03-04 04:13:15 PST
(In reply to
comment #3
)
> (From update of
attachment 191175
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=191175&action=review
> > >> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:114 > >> + m_ended = true; > > > > To the actual reviewer: > > The sources are filtered in Source/WebCore/Modules/mediastream/MediaStream.cpp in the function processTrack so when the execution comes to this stage all ended tracks have been ignored. > > Can we add some ASSERT() about this?
The ended status is not fatal to MediaStreamDescriptor object, so maybe it is not necessary to do that.
> > >> LayoutTests/fast/mediastream/MediaStreamConstructor.html:88 > >> + var ended = arguments[3]?arguments[3]:false; > > > > Nit: I would prefer ended to be added as a proper argument. Also spaces around the operators is the norm. > > Agreed.
I just wanted to express the default value of ended attribute. Anyway, I will fix it.
Li Yin
Comment 5
2013-03-04 04:21:16 PST
Created
attachment 191202
[details]
Patch
Kentaro Hara
Comment 6
2013-03-04 04:37:17 PST
Comment on
attachment 191202
[details]
Patch OK, thank you and tommy!
WebKit Review Bot
Comment 7
2013-03-04 05:56:56 PST
Comment on
attachment 191202
[details]
Patch Clearing flags on attachment: 191202 Committed
r144623
: <
http://trac.webkit.org/changeset/144623
>
WebKit Review Bot
Comment 8
2013-03-04 05:56:59 PST
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.
Top of Page
Format For Printing
XML
Clone This Bug