Delete unused and almost unused frame-related methods.

Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

TBR=tkchin@webrtc.org # Refactoring affecting RTCVideoFrame
BUG=webrtc:5682
Committed: https://crrev.com/76270de4bc2dac188f10f805e6e2fb86693ef864
Cr-Commit-Position: refs/heads/master@{#13183}

[nisse-webrtc, 2016-06-14 12:21:10]

nisse@webrtc.org changed reviewers:
+ pbos@webrtc.org, perkj@webrtc.org

[nisse-webrtc, 2016-06-14 12:21:11]

This cl deletes a couple of more methods. It would be nice if one of you could
start one or a few of the new shiny chromium try bots. I get Access denied when
I try to do that.

[pbos-webrtc, 2016-06-14 13:47:05]

Check with kjellander@ to see what's up with that trybot thing.

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
webrtc/modules/video_coding/video_sender.cc:291: // TODO(nisse): Do we need to
propagate the meta data?
what meta data? I have no idea what this comment means.

[nisse-webrtc, 2016-06-14 14:32:14]

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
webrtc/modules/video_coding/video_sender.cc:291: // TODO(nisse): Do we need to
propagate the meta data?
On 2016/06/14 13:47:05, pbos-webrtc wrote:
> what meta data? I have no idea what this comment means.

Frame timestamps and rotation. Does the encoder care about them? Before this cl,
rotation was discarded, propagating it below is a change which I'm not 100% sure
is correct.

If you explain it, I'll happily remove the comment.

[pbos-webrtc, 2016-06-14 14:34:15]

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
webrtc/modules/video_coding/video_sender.cc:291: // TODO(nisse): Do we need to
propagate the meta data?
On 2016/06/14 14:32:14, nisse-webrtc wrote:
> On 2016/06/14 13:47:05, pbos-webrtc wrote:
> > what meta data? I have no idea what this comment means.
> 
> Frame timestamps and rotation. Does the encoder care about them? Before this
cl,
> rotation was discarded, propagating it below is a change which I'm not 100%
sure
> is correct.
> 
> If you explain it, I'll happily remove the comment.

It should, timestamp and rotation needs to be forwarded through the encoder. I
think you fixed a bug here.

[nisse-webrtc, 2016-06-15 07:49:20]

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_coding/v...
webrtc/modules/video_coding/video_sender.cc:291: // TODO(nisse): Do we need to
propagate the meta data?
On 2016/06/14 14:34:15, pbos-webrtc wrote:
> On 2016/06/14 14:32:14, nisse-webrtc wrote:
> > On 2016/06/14 13:47:05, pbos-webrtc wrote:
> > > what meta data? I have no idea what this comment means.
> > 
> > Frame timestamps and rotation. Does the encoder care about them? Before this
> cl,
> > rotation was discarded, propagating it below is a change which I'm not 100%
> sure
> > is correct.
> > 
> > If you explain it, I'll happily remove the comment.
> 
> It should, timestamp and rotation needs to be forwarded through the encoder. I
> think you fixed a bug here.

I see, deleted the comment. And on second look, I don't think behaviour is
changed here, the old ConvertNativeToI420Frame did copy rotation.

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_processi...
File webrtc/modules/video_processing/spatial_resampler.cc (right):

https://codereview.webrtc.org/2065733003/diff/1/webrtc/modules/video_processi...
webrtc/modules/video_processing/spatial_resampler.cc:64: inFrame.rotation());
On the other hand, at this place the old version discarded rotation info, and
the new version copies it.

[perkj_webrtc, 2016-06-15 20:43:12]

lgtm provided the below is addressed.

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:964: 0,
webrtc::kVideoRotation_0);
nit: what does 0 mean here? can your write 0 /* parameter name */, ...

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/video_sender.cc:294: RTC_CHECK(converted_buffer)
in b/28636240 we are asked to replace this check with a warning. Can you please
do that?

[pbos-webrtc, 2016-06-15 20:46:30]

lgtm

[nisse-webrtc, 2016-06-16 07:22:28]

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc (right):

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc:964: 0,
webrtc::kVideoRotation_0);
On 2016/06/15 20:43:12, perkj_webrtc wrote:
> nit: what does 0 mean here? can your write 0 /* parameter name */, ...

Adding /* render_time_ms */. This code only sets the 90kHz timestamp.

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/20001/webrtc/modules/video_codi...
webrtc/modules/video_coding/video_sender.cc:294: RTC_CHECK(converted_buffer)
On 2016/06/15 20:43:12, perkj_webrtc wrote:
> in b/28636240 we are asked to replace this check with a warning. Can you
please
> do that?

Not sure what to do instead. For now, log a warning, and pass the original
native frame on to the encoder.

[nisse-webrtc, 2016-06-16 07:22:36]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-16 07:22:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2065733003/40001

[commit-bot: I haz the power, 2016-06-16 07:26:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-16 07:26:13]

Dry run: Try jobs failed on following builders:
  android_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/14211)

[nisse-webrtc, 2016-06-16 08:57:53]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-16 08:57:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2065733003/140001

[nisse-webrtc, 2016-06-16 08:59:24]

Please have another look, I forgot android and h264 in the first round, but they
also needed to be updated.

[commit-bot: I haz the power, 2016-06-16 08:59:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-16 08:59:48]

Dry run: Try jobs failed on following builders:
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/8309)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/10625)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/10550)

[nisse-webrtc, 2016-06-16 09:34:36]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-16 09:34:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2065733003/180001

[nisse-webrtc, 2016-06-16 09:41:34]

nisse@webrtc.org changed reviewers:
+ marpan@webrtc.org, tkchin@webrtc.org

[nisse-webrtc, 2016-06-16 09:41:35]

Owner's approval needed for a few files.

marpan: spatial_resampler.cc

tkchin: RTCVideoFrame.mm

[commit-bot: I haz the power, 2016-06-16 09:45:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-16 09:45:51]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)

[pbos-webrtc, 2016-06-16 11:21:43]

lgtm

https://codereview.webrtc.org/2065733003/diff/180001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/180001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_sender.cc:295: LOG(LS_WARNING) << "Frame
conversion failed, dropping frame.";
LS_ERROR

[nisse-webrtc, 2016-06-16 13:13:07]

https://codereview.webrtc.org/2065733003/diff/180001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_sender.cc (right):

https://codereview.webrtc.org/2065733003/diff/180001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_sender.cc:295: LOG(LS_WARNING) << "Frame
conversion failed, dropping frame.";
On 2016/06/16 11:21:43, pbos-webrtc wrote:
> LS_ERROR

Done.

[marpan2, 2016-06-16 14:22:00]

lgtm

[nisse-webrtc, 2016-06-16 14:48:27]

Description was changed from

==========
Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

BUG=webrtc:5682
==========

to

==========
Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

TBR=tkchin@webrtc.org # Refactoring affecting RTCVideoFrame 
BUG=webrtc:5682
==========

[nisse-webrtc, 2016-06-16 14:48:38]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-06-16 14:48:39]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@webrtc.org,
pbos@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2065733003/#ps200001 (title:
"Change log level to LS_ERROR.")

[commit-bot: I haz the power, 2016-06-16 14:48:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2065733003/200001

[commit-bot: I haz the power, 2016-06-16 15:04:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-16 15:04:49]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)

[nisse-webrtc, 2016-06-17 07:43:31]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-06-17 07:43:32]

The patchset sent to the CQ was uploaded after l-g-t-m from marpan@webrtc.org,
pbos@webrtc.org, perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2065733003/#ps220001 (title:
"Rebase. Eliminate call to set_video_frame_buffer.")

[commit-bot: I haz the power, 2016-06-17 07:43:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2065733003/220001

[commit-bot: I haz the power, 2016-06-17 09:00:16]

Description was changed from

==========
Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

TBR=tkchin@webrtc.org # Refactoring affecting RTCVideoFrame 
BUG=webrtc:5682
==========

to

==========
Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

TBR=tkchin@webrtc.org # Refactoring affecting RTCVideoFrame 
BUG=webrtc:5682
==========

[commit-bot: I haz the power, 2016-06-17 09:00:18]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-06-17 09:00:27]

Description was changed from

==========
Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

TBR=tkchin@webrtc.org # Refactoring affecting RTCVideoFrame 
BUG=webrtc:5682
==========

to

==========
Delete unused and almost unused frame-related methods.

webrtc::VideoFrame::set_video_frame_buffer
webrtc::VideoFrame::ConvertNativeToI420Frame

cricket::WebRtcVideoFrame::InitToBlack

VideoFrameBuffer::data
VideoFrameBuffer::stride
VideoFrameBuffer::MutableData

TBR=tkchin@webrtc.org # Refactoring affecting RTCVideoFrame
BUG=webrtc:5682

Committed: https://crrev.com/76270de4bc2dac188f10f805e6e2fb86693ef864
Cr-Commit-Position: refs/heads/master@{#13183}
==========

[commit-bot: I haz the power, 2016-06-17 09:00:28]

Patchset 12 (id:??) landed as
https://crrev.com/76270de4bc2dac188f10f805e6e2fb86693ef864
Cr-Commit-Position: refs/heads/master@{#13183}

[nisse-webrtc, 2016-06-17 09:54:55]

A revert of this CL (patchset #12 id:220001) has been created in
https://codereview.webrtc.org/2076113002/ by nisse@webrtc.org.

The reason for reverting is: Breaks downstream applications which inherits
webrtc::VideoFrameBuffer and tries to override deleted methods data(), stride()
and MutableData()..