Change a disabled MediaStreamVideoTrack to output black video frames for each incoming frame.

Change a disabled MediaStreamVideoTrack to output black video frames for each incoming frame.
The frame provided to the track is unchanged. Instead a black frame with the same timestamp is created and forwarded to the sinks.
Please see the bug and the discussion in https://groups.google.com/forum/#!topic/discuss-webrtc/nZWA0prohio for rational.

BUG=398844
TEST= open https://apprtc.appspot.com in two tabs to create a call. Hit Ctrl-shift-e in the first tab to disable the local video track. Notice that local video is black.  Open the second tab and notice that the remote video is black.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287787

[perkj_chrome, 2014-07-31 20:20:55]

Hej
Can you please review? If its not too much work I will try to land this while I
am ooo. Otherwise I might ask for help.

[tommi (sloooow) - chröme, 2014-08-05 09:50:46]

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
File content/renderer/media/media_stream_video_track.cc (right):

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track.cc:23: // is disabled, a black
frame is instead forwarded to the sinks.
is fps relevant in this situation?  I.e. if the track was originally opened as
60fps, will it deliver 60 black frames ps when it's disabled?

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track.cc:57: const
scoped_refptr<media::VideoFrame>& GetBlackFrame(
can you document this function?  The use of scoped_refptr, singular "black
frame" and the |original_frame| parameter, raise some questions.

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track.cc:156: const
scoped_refptr<media::VideoFrame>& frame) {
different parameter name here than from the header... change to original_frame?
(or should it be reference_frame?)

Is there any chance that we could ping pong between resolutions and continually
be recreating black frames?

As an alternate possibility, would it make sense to blacken out the original
frame, or would that affect other tracks that share the same source?

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
File content/renderer/media/media_stream_video_track_unittest.cc (right):

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track_unittest.cc:22: const uint8
kColorValue = 0xFF;
nit: maybe we should pick a different, harder to guess by accident, value for
'color'.

[perkj_chrome, 2014-08-06 12:32:22]

PTAL

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
File content/renderer/media/media_stream_video_track.cc (right):

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track.cc:23: // is disabled, a black
frame is instead forwarded to the sinks.
On 2014/08/05 09:50:46, tommi wrote:
> is fps relevant in this situation?  I.e. if the track was originally opened as
> 60fps, will it deliver 60 black frames ps when it's disabled?

Yes. That is intentional. To my understanding - the only difference to the sinks
is that a black frame is received instead.

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track.cc:57: const
scoped_refptr<media::VideoFrame>& GetBlackFrame(
On 2014/08/05 09:50:46, tommi wrote:
> can you document this function?  The use of scoped_refptr, singular "black
> frame" and the |original_frame| parameter, raise some questions.

Done.

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track.cc:156: const
scoped_refptr<media::VideoFrame>& frame) {
On 2014/08/05 09:50:46, tommi wrote:
> different parameter name here than from the header... change to
original_frame?
> (or should it be reference_frame?)
> 

done:

> Is there any chance that we could ping pong between resolutions and
continually
> be recreating black frames?
> 
Only if the source do that for some reason- and that sounds like a bug in the
source if it happens frequently. (Changing video resolution should be possible
with applyConstraints one day). 

> As an alternate possibility, would it make sense to blacken out the original
> frame, or would that affect other tracks that share the same source?

That is correct. It would affect all tracks that use the same source. So I think
this is the correct place.

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
File content/renderer/media/media_stream_video_track_unittest.cc (right):

https://codereview.chromium.org/432903002/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_video_track_unittest.cc:22: const uint8
kColorValue = 0xFF;
On 2014/08/05 09:50:46, tommi wrote:
> nit: maybe we should pick a different, harder to guess by accident, value for
> 'color'.

Done.

[tommi (sloooow) - chröme, 2014-08-06 13:16:58]

lgtm

[perkj_chrome, 2014-08-06 14:16:51]

The CQ bit was checked by perkj@chromium.org

[commit-bot: I haz the power, 2014-08-06 14:17:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/perkj@chromium.org/432903002/40001

[commit-bot: I haz the power, 2014-08-06 16:45:02]

Change committed as 287787