Delete old RTCVideoDecoder code path.

Delete old RTCVideoDecoder code path.

Remove leftover codepath from the pre-WebMediaPlayerMS days.

BUG=chromium:177572
TEST=Try apprtc.appspot.com/?debug=loopback on link.

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

[Ami GONE FROM CHROMIUM, 2013-02-26 23:16:55]

+wjia who has a lot more context on this

LGTM % nits and please wait for Wei's LGTM as well.

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/med...
File content/renderer/media/media_stream_impl.cc (right):

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/med...
content/renderer/media/media_stream_impl.cc:23: #include
"media/base/video_decoder.h"
drop if GVD() goes away below

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/med...
content/renderer/media/media_stream_impl.cc:251:
scoped_refptr<media::VideoDecoder> MediaStreamImpl::GetVideoDecoder(
Why not delete this method?

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc...
File content/renderer/media/rtc_video_decoder_unittest.cc (left):

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc...
content/renderer/media/rtc_video_decoder_unittest.cc:175: class
RTCVideoDecoderTest : public testing::Test {
I guess now'd be a good time to inspect the test coverage of RTCVideoRenderer
and make sure it's up to snuff (if it's not, add a TODO there pointing at a
now-ish svn rev of this file to be a guide)

[wjia(left Chromium), 2013-02-26 23:29:33]

adding edjee@ since there was a request to keep WebMediaPlayerImpl path from gTV
team.

Will start reviewing after getting confirmation from gTV team.

[wuchengli, 2013-02-27 01:45:41]

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/med...
File content/renderer/media/media_stream_impl.cc (right):

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/med...
content/renderer/media/media_stream_impl.cc:23: #include
"media/base/video_decoder.h"
On 2013/02/26 23:16:55, Ami Fischman wrote:
> drop if GVD() goes away below
Removed.

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/med...
content/renderer/media/media_stream_impl.cc:251:
scoped_refptr<media::VideoDecoder> MediaStreamImpl::GetVideoDecoder(
On 2013/02/26 23:16:55, Ami Fischman wrote:
> Why not delete this method?
Removed.

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc...
File content/renderer/media/rtc_video_decoder_unittest.cc (left):

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc...
content/renderer/media/rtc_video_decoder_unittest.cc:175: class
RTCVideoDecoderTest : public testing::Test {
I'm not familiar with svn. How to get the latest svn rev of a file? I tried "svn
blame content/renderer/media/rtc_video_decoder_unittest.cc". The error message
is "svn: 'content/renderer/media' is not a working copy". I also tried "git svn
info" and it got stuck.
On 2013/02/26 23:16:55, Ami Fischman wrote:
> I guess now'd be a good time to inspect the test coverage of RTCVideoRenderer
> and make sure it's up to snuff (if it's not, add a TODO there pointing at a
> now-ish svn rev of this file to be a guide)

[edjee, 2013-02-27 02:04:01]

Hi, if this change will not be cherrypicked in M26 branch, we're fine.

Thanks a lot.

[Ami GONE FROM CHROMIUM, 2013-02-27 02:15:26]

> https://codereview.chromium.**org/12320078/diff/1005/**
>
content/renderer/media/rtc_**video_decoder_unittest.cc#**oldcode175<https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc_video_decoder_unittest.cc#oldcode175>
> content/renderer/media/rtc_**video_decoder_unittest.cc:175: class
> RTCVideoDecoderTest : public testing::Test {
> I'm not familiar with svn. How to get the latest svn rev of a file? I
> tried "svn blame content/renderer/media/rtc_**video_decoder_unittest.cc".
> The error message is "svn: 'content/renderer/media' is not a working
> copy". I also tried "git svn info" and it got stuck.


This is a reasonable pointer to use, and includes all the information
needed to recreate from VCS if ViewVC changes:
http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/rtc_vi...

Wei: do you want to weigh in with a judgement about there being anything in
this file worth salvaging for testing the RTCVideoRenderer world?

[wuchengli, 2013-02-27 02:28:43]

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc...
File content/renderer/media/rtc_video_decoder_unittest.cc (left):

https://codereview.chromium.org/12320078/diff/1005/content/renderer/media/rtc...
content/renderer/media/rtc_video_decoder_unittest.cc:175: class
RTCVideoDecoderTest : public testing::Test {
So the revision is 180591. Right?

I forgot to say there is no test for RTCVideoRenderer.
On 2013/02/27 01:45:41, wuchengli wrote:
> I'm not familiar with svn. How to get the latest svn rev of a file? I tried
"svn
> blame content/renderer/media/rtc_video_decoder_unittest.cc". The error message
> is "svn: 'content/renderer/media' is not a working copy". I also tried "git
svn
> info" and it got stuck.
> On 2013/02/26 23:16:55, Ami Fischman wrote:
> > I guess now'd be a good time to inspect the test coverage of
RTCVideoRenderer
> > and make sure it's up to snuff (if it's not, add a TODO there pointing at a
> > now-ish svn rev of this file to be a guide)
>

[wuchengli, 2013-02-27 02:31:11]

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
File content/renderer/media/rtc_video_renderer.h (right):

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
content/renderer/media/rtc_video_renderer.h:29: //   revision 180591 for
reference.
I just added revision number. Do you want the full url
http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/rtc_vi...
here?

[wjia(left Chromium), 2013-02-27 02:43:54]

lgtm with nits.

Ami: that's a good point. I think part of the tests in
rtc_video_decoder_unittest.cc can be borrowed to create unit tests for
RTCVideoRenderer, e.g., SetSize and RenderFrame.

https://codereview.chromium.org/12320078/diff/5037/webkit/media/media_stream_...
File webkit/media/media_stream_client.h (right):

https://codereview.chromium.org/12320078/diff/5037/webkit/media/media_stream_...
webkit/media/media_stream_client.h:19: class VideoDecoder;
nit: remove this.

https://codereview.chromium.org/12320078/diff/5037/webkit/media/webmediaplaye...
File webkit/media/webmediaplayer_params.h (right):

https://codereview.chromium.org/12320078/diff/5037/webkit/media/webmediaplaye...
webkit/media/webmediaplayer_params.h:18: class MediaStreamClient;
nit: remove this line.

[scherkus (not reviewing), 2013-02-27 06:00:16]

I <3 this change -- LGTM w/ nits

Thanks for doing this!

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
File content/renderer/media/rtc_video_renderer.h (right):

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
content/renderer/media/rtc_video_renderer.h:29: //   revision 180591 for
reference.
nit: don't bother indenting

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
content/renderer/media/rtc_video_renderer.h:29: //   revision 180591 for
reference.
On 2013/02/27 02:31:11, wuchengli wrote:
> I just added revision number. Do you want the full url
>
http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/rtc_vi...
> here?

link is fine

https://codereview.chromium.org/12320078/diff/11011/webkit/media/filter_helpe...
File webkit/media/filter_helpers.cc (right):

https://codereview.chromium.org/12320078/diff/11011/webkit/media/filter_helpe...
webkit/media/filter_helpers.cc:19: #include "webkit/media/media_stream_client.h"
can we remove this?

[wuchengli, 2013-02-27 21:37:41]

I can write a unit test for RTCVideoRenderer in the next cl if you think it's
good to have it.

https://codereview.chromium.org/12320078/diff/5037/webkit/media/media_stream_...
File webkit/media/media_stream_client.h (right):

https://codereview.chromium.org/12320078/diff/5037/webkit/media/media_stream_...
webkit/media/media_stream_client.h:19: class VideoDecoder;
On 2013/02/27 02:43:54, wjia wrote:
> nit: remove this.

Done.

https://codereview.chromium.org/12320078/diff/5037/webkit/media/webmediaplaye...
File webkit/media/webmediaplayer_params.h (right):

https://codereview.chromium.org/12320078/diff/5037/webkit/media/webmediaplaye...
webkit/media/webmediaplayer_params.h:18: class MediaStreamClient;
On 2013/02/27 02:43:54, wjia wrote:
> nit: remove this line.

Done.

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
File content/renderer/media/rtc_video_renderer.h (right):

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
content/renderer/media/rtc_video_renderer.h:29: //   revision 180591 for
reference.
On 2013/02/27 06:00:16, scherkus wrote:
> nit: don't bother indenting

Done.

https://codereview.chromium.org/12320078/diff/11011/content/renderer/media/rt...
content/renderer/media/rtc_video_renderer.h:29: //   revision 180591 for
reference.
On 2013/02/27 06:00:16, scherkus wrote:
> On 2013/02/27 02:31:11, wuchengli wrote:
> > I just added revision number. Do you want the full url
> >
>
http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/rtc_vi...
> > here?
> 
> link is fine

Done.

https://codereview.chromium.org/12320078/diff/11011/webkit/media/filter_helpe...
File webkit/media/filter_helpers.cc (right):

https://codereview.chromium.org/12320078/diff/11011/webkit/media/filter_helpe...
webkit/media/filter_helpers.cc:19: #include "webkit/media/media_stream_client.h"
On 2013/02/27 06:00:16, scherkus wrote:
> can we remove this?

Done.

[wuchengli, 2013-02-28 01:23:25]

Please take another look. Thanks.

[scherkus (not reviewing), 2013-02-28 01:33:35]

I believe you can also delete the following:

media/filters/dummy_demuxer.{cc,h}
media/filters/video_frame_generator.{cc,h}

[wuchengli, 2013-02-28 02:03:08]

On 2013/02/28 01:33:35, scherkus wrote:
> I believe you can also delete the following:
> 
> media/filters/dummy_demuxer.{cc,h}
> media/filters/video_frame_generator.{cc,h}
That's right! Removed.

[scherkus (not reviewing), 2013-02-28 02:22:13]

lgtm

[commit-bot: I haz the power, 2013-02-28 17:53:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wuchengli@chromium.org/12320078/20004

[commit-bot: I haz the power, 2013-02-28 17:53:24]

Presubmit check for 12320078-20004 failed and returned exit status 1.

INFO:root:Found 21 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  content/renderer/media/rtc_video_renderer.h, line 29, 131 chars

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    webkit/support/test_media_stream_client.h
    content/content_renderer.gypi
    webkit/support/webkit_support.cc
    content/browser/renderer_host/render_process_host_impl.cc
    webkit/support/test_media_stream_client.cc
    content/content_tests.gypi
    content/renderer/render_view_impl.cc
    content/public/common/content_switches.cc
    content/public/common/content_switches.h
    webkit/tools/test_shell/test_webview_delegate.cc

Presubmit checks took 2.1s to calculate.

[wuchengli, 2013-02-28 18:16:22]

brettw: please owners.

Thanks.

[brettw, 2013-02-28 21:38:02]

LGTM rubberstamp

[commit-bot: I haz the power, 2013-02-28 22:13:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wuchengli@chromium.org/12320078/20004

[commit-bot: I haz the power, 2013-02-28 22:13:48]

Presubmit check for 12320078-20004 failed and returned exit status 1.

INFO:root:Found 21 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  content/renderer/media/rtc_video_renderer.h, line 29, 131 chars

Presubmit checks took 2.3s to calculate.

[commit-bot: I haz the power, 2013-03-01 00:58:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wuchengli@chromium.org/12320078/33002

[commit-bot: I haz the power, 2013-03-01 04:17:34]

Change committed as 185460