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
Patch (9.12 KB, patch)
2013-03-04 04:21 PST, Li Yin
no flags
Li Yin
Comment 1 2013-03-04 01:37:43 PST
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
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.