Switch event type when a capture device has been stopped from the render process. This make sure th…

Switch event type when a capture device has been stopped from the render process. This make sure that RtcVideoCapturer is notified when the screen share Stop button has been called or a LocalMediaStream has been stopped.

https://code.google.com/p/webrtc/issues/detail?id=960

BUG=226400
TEST= Enable logging on rtc_video_capture. Navigate to http://src.chromium.org/chrome/trunk/src/chrome/test/data/webrtc/manual/peerconnection.html. Press the Request GetUserMedia button. Open up a JS console and type gLocalStream.stop() and verify that RtcVideoCapturer::OnStateChange 0 is triggered.
On Linux, Repeat starting the video camera. Unplug the USB device and verify that RtcVideoCapturer::OnStateChange 2 is triggered.

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

[perkj_chrome, 2013-04-04 12:18:07]

Hej Wei- Can you please do a code review?

[wjia(left Chromium), 2013-04-04 17:05:11]

https://codereview.chromium.org/13616004/diff/2001/content/browser/renderer_h...
File
content/browser/renderer_host/media/video_capture_controller_event_handler.h
(right):

https://codereview.chromium.org/13616004/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/media/video_capture_controller_event_handler.h:48:
// The capture session has ended and no more more frames will be sent.
nit: remove the extra "more".

https://codereview.chromium.org/13616004/diff/2001/content/renderer/media/rtc...
File content/renderer/media/rtc_video_capture_delegate.cc (right):

https://codereview.chromium.org/13616004/diff/2001/content/renderer/media/rtc...
content/renderer/media/rtc_video_capture_delegate.cc:114:
state_callback_.Run(got_first_frame_ ? CAPTURE_STOPPED : CAPTURE_FAILED);
OnRemoved could be called after either OnError or OnStopped. In those cases, do
you still need signal the state change?

[perkj_chrome, 2013-04-05 09:19:35]

https://codereview.chromium.org/13616004/diff/2001/content/browser/renderer_h...
File
content/browser/renderer_host/media/video_capture_controller_event_handler.h
(right):

https://codereview.chromium.org/13616004/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/media/video_capture_controller_event_handler.h:48:
// The capture session has ended and no more more frames will be sent.
On 2013/04/04 17:05:11, wjia wrote:
> nit: remove the extra "more".

Done.

https://codereview.chromium.org/13616004/diff/2001/content/renderer/media/rtc...
File content/renderer/media/rtc_video_capture_delegate.cc (right):

https://codereview.chromium.org/13616004/diff/2001/content/renderer/media/rtc...
content/renderer/media/rtc_video_capture_delegate.cc:114:
state_callback_.Run(got_first_frame_ ? CAPTURE_STOPPED : CAPTURE_FAILED);
On 2013/04/04 17:05:11, wjia wrote:
> OnRemoved could be called after either OnError or OnStopped. In those cases,
do
> you still need signal the state change?
 right - removed onerror and only listen to OnRemoved.

[wjia(left Chromium), 2013-04-05 18:39:17]

https://codereview.chromium.org/13616004/diff/8001/content/renderer/media/rtc...
File content/renderer/media/rtc_video_capture_delegate.cc (right):

https://codereview.chromium.org/13616004/diff/8001/content/renderer/media/rtc...
content/renderer/media/rtc_video_capture_delegate.cc:110:
state_callback_.Run(got_first_frame_ ? CAPTURE_STOPPED : CAPTURE_FAILED);
This could still have problem since OnRemoved is called immediately after
onStopped too
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...).
For a session, when capture is stopped normally, this function would signal
CAPTURE_FAILED. Is that desired?

The possible fix would be remember state when OnStopped/OnError is called. Then
check that state here.

[perkj_chrome, 2013-04-08 11:15:54]

https://codereview.chromium.org/13616004/diff/8001/content/renderer/media/rtc...
File content/renderer/media/rtc_video_capture_delegate.cc (right):

https://codereview.chromium.org/13616004/diff/8001/content/renderer/media/rtc...
content/renderer/media/rtc_video_capture_delegate.cc:110:
state_callback_.Run(got_first_frame_ ? CAPTURE_STOPPED : CAPTURE_FAILED);
On 2013/04/05 18:39:17, wjia wrote:
> This could still have problem since OnRemoved is called immediately after
> onStopped too
>
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...).
> For a session, when capture is stopped normally, this function would signal
> CAPTURE_FAILED. Is that desired?
> 
> The possible fix would be remember state when OnStopped/OnError is called.
Then
> check that state here.

Do you know why we have have got_first_frame_ ? Why not replace it with

RtcVideoCaptureDelegate::OnError {
got_error_ = true;
}

And do 
void RtcVideoCaptureDelegate::OnRemovedOnCaptureThread(
  if (!state_callback_.is_null()) 
state_callback_.Run(got_error_ ? CAPTURE_FAILED : CAPTURE_STOPPED);

[wjia(left Chromium), 2013-04-08 14:21:47]

On 2013/04/08 11:15:54, perkj wrote:
>
https://codereview.chromium.org/13616004/diff/8001/content/renderer/media/rtc...
> File content/renderer/media/rtc_video_capture_delegate.cc (right):
> 
>
https://codereview.chromium.org/13616004/diff/8001/content/renderer/media/rtc...
> content/renderer/media/rtc_video_capture_delegate.cc:110:
> state_callback_.Run(got_first_frame_ ? CAPTURE_STOPPED : CAPTURE_FAILED);
> On 2013/04/05 18:39:17, wjia wrote:
> > This could still have problem since OnRemoved is called immediately after
> > onStopped too
> >
>
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...).
> > For a session, when capture is stopped normally, this function would signal
> > CAPTURE_FAILED. Is that desired?
> > 
> > The possible fix would be remember state when OnStopped/OnError is called.
> Then
> > check that state here.
> 
> Do you know why we have have got_first_frame_ ? Why not replace it with
> 
> RtcVideoCaptureDelegate::OnError {
> got_error_ = true;
> }
> 
> And do 
> void RtcVideoCaptureDelegate::OnRemovedOnCaptureThread(
>   if (!state_callback_.is_null()) 
> state_callback_.Run(got_error_ ? CAPTURE_FAILED : CAPTURE_STOPPED);

IIRC, it's because of the definition of those states in
third_party/libjingle/source/talk/media/base/videocapturer.h (chrome code search
somehow can't find this file).

[perkj_chrome, 2013-04-09 09:22:24]

PTAL

[wjia(left Chromium), 2013-04-09 16:22:51]

lgtm

[commit-bot: I haz the power, 2013-04-12 08:12:01]

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

[commit-bot: I haz the power, 2013-04-12 12:31:27]

Change committed as 193911