Bug 151299 - [MediaStream] VideoTrack should respond to MediaStreamTrack state changes
Summary: [MediaStream] VideoTrack should respond to MediaStreamTrack state changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-11-15 17:01 PST by Eric Carlson
Modified: 2016-01-04 13:14 PST (History)
7 users (show)

See Also:


Attachments
Proposed patch. (44.93 KB, patch)
2015-11-15 17:47 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing. (44.68 KB, patch)
2015-11-16 14:21 PST, Eric Carlson
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (721.68 KB, application/zip)
2015-11-16 15:11 PST, Build Bot
no flags Details
Updated patch for landing. (46.56 KB, patch)
2015-11-16 17:32 PST, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2015-11-15 17:01:23 PST
The specs says: 

When the MediaStream state moves from the active to the inactive state, the User Agent must raise an ended event on the HTMLMediaElement and set its ended attribute to true. Note that once ended equals true the HTMLMediaElement will not play media...
Comment 1 Eric Carlson 2015-11-15 17:47:22 PST
Created attachment 265562 [details]
Proposed patch.
Comment 2 Jer Noble 2015-11-16 13:24:15 PST
Comment on attachment 265562 [details]
Proposed patch.

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

r=me with nits

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:216
> +                static CGFloat components[4] = { 0, 0, 0, 1 };
> +                static CGColorRef blackColor = CGColorCreate(CGColorSpaceCreateDeviceRGB(), components);
> +                m_videoBackgroundLayer.get().contents = nil;
> +                m_videoBackgroundLayer.get().backgroundColor = blackColor;

You could also use backgroundColor = cachedCGColor(Color::black); here.

> Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp:172
> +        context.scale(FloatSize(1, -1));

Is it really important to flip the scale if you're just filling with black?
Comment 3 Eric Carlson 2015-11-16 14:21:07 PST
Created attachment 265617 [details]
Patch for landing.
Comment 4 Eric Carlson 2015-11-16 14:21:41 PST
(In reply to comment #2)
> Comment on attachment 265562 [details]
> Proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265562&action=review
> 
> r=me with nits
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:216
> > +                static CGFloat components[4] = { 0, 0, 0, 1 };
> > +                static CGColorRef blackColor = CGColorCreate(CGColorSpaceCreateDeviceRGB(), components);
> > +                m_videoBackgroundLayer.get().contents = nil;
> > +                m_videoBackgroundLayer.get().backgroundColor = blackColor;
> 
> You could also use backgroundColor = cachedCGColor(Color::black); here.
> 
Good idea, fixed.

> > Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp:172
> > +        context.scale(FloatSize(1, -1));
> 
> Is it really important to flip the scale if you're just filling with black?
>
Nope, fixed.
Comment 5 Build Bot 2015-11-16 15:11:26 PST
Comment on attachment 265617 [details]
Patch for landing.

Attachment 265617 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/438466

New failing tests:
fast/mediastream/MediaStream-video-element.html
Comment 6 Build Bot 2015-11-16 15:11:28 PST
Created attachment 265627 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 7 Eric Carlson 2015-11-16 17:32:41 PST
Created attachment 265644 [details]
Updated patch for landing.
Comment 8 WebKit Commit Bot 2015-11-16 18:52:21 PST
Comment on attachment 265644 [details]
Updated patch for landing.

Clearing flags on attachment: 265644

Committed r192503: <http://trac.webkit.org/changeset/192503>