Bug 122358 – Create MediaStream object with ended attribute set if all tracks that are being used on its creation are ended

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Thiago de Barros Lacerda]

Spec tells that if all tracks that belongs to a new MediaStream object being created are ended, then the ended attribute of this MediaStream must be set to true.
http://www.w3.org/TR/mediacapture-streams/#widl-MediaStream-ended

[Thiago de Barros Lacerda]

Created attachment 213882 [details]
Patch

[Thiago de Barros Lacerda]

Created attachment 213917 [details]
Patch

[Thiago de Barros Lacerda]

Created attachment 213920 [details]
Patch

[Thiago de Barros Lacerda]

Created attachment 213921 [details]
Patch

[Jer Noble]

Comment on attachment 213921 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=213921&action=review

A few nits:

> Source/WebCore/Modules/mediastream/MediaStream.cpp:104
> -    for (size_t i = 0; i < tracks.size(); ++i)
> +    bool ended = true;
> +    for (size_t i = 0; i < tracks.size(); ++i) {
> +        ended &= tracks[i]->ended();
>          processTrack(tracks[i].get(), tracks[i]->kind() == "audio" ? audioSources : videoSources);
> +    }
>  
> -    return createFromSourceVectors(context, audioSources, videoSources);
> +    return createFromSourceVectors(context, audioSources, videoSources, ended);

Nit: I would have named this "allEnded" just to make clear what you're doing with this variable.

> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:55
> -    static PassRefPtr<MediaStreamDescriptor> create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources);
> +    static PassRefPtr<MediaStreamDescriptor> create(const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, bool ended);

I don't particularly like adding new "bool" parameters to creation functions.  It's hard to figure out what that true or false parameter means from the a call site.  The preferred way to do this is with an enum:

enum EndedAtCreationFlags { IsNotEnded, IsEnded };
static PassRefPtr<MediaStream> createFromSourceVectors(ScriptExecutionContext* context, const MediaStreamSourceVector& audioSources, const MediaStreamSourceVector& videoSources, EndedAtCreationFlags flags)

Then the EndedAtCreationFlags type would be used elsewhere where this method (or similar ones) are called.

> Source/WebCore/platform/mock/MockMediaStreamCenter.cpp:236
> -    client->didCreateStream(MediaStreamDescriptor::create(audioSources, videoSources));
> +    client->didCreateStream(MediaStreamDescriptor::create(audioSources, videoSources, false));

...Then this line becomes much more understandable:

client->didCreateStream(MediaStreamDescriptor::create(audioSources, videoSources, MediaStreamDescriptor::IsNotEnded));

There are a few examples like this in HTMLMediaElement.h

[Thiago de Barros Lacerda]

Created attachment 213936 [details]
Patch

[Thiago de Barros Lacerda]

Adding Enum on MediaStreamDescriptor create method

[Jer Noble]

Comment on attachment 213936 [details]
Patch

Looks good! r=me.

[WebKit Commit Bot]

Comment on attachment 213936 [details]
Patch

Clearing flags on attachment: 213936

Committed r157268: <http://trac.webkit.org/changeset/157268>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.