Delete support for receiving RTCP RPSI and SLI messages.

Delete support for receiving RTCP RPSI and SLI message.

This code has been unused for years, and at least the RTCP RSPI sending
logic appears broken.

This cl is part 3, following 

  https://codereview.webrtc.org/2746413003 (delete sending)
  https://codereview.webrtc.org/2753783002 (delete vp8 feedback mode)

BUG=webrtc:7338
Review-Url: https://codereview.webrtc.org/2742383004
Cr-Commit-Position: refs/heads/master@{#17342}
Committed: https://chromium.googlesource.com/external/webrtc/+/25d0bdc1bcbd78adabe5dac4ff965434cd83a41f

[nisse-webrtc, 2017-03-14 11:47:26]

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

[commit-bot: I haz the power, 2017-03-14 11:47:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[nisse-webrtc, 2017-03-14 11:48:32]

nisse@webrtc.org changed reviewers:
+ danilchap@webrtc.org, stefan@webrtc.org

[nisse-webrtc, 2017-03-14 11:48:34]

PTAL, this ended up as a pretty large deletion.

[commit-bot: I haz the power, 2017-03-14 11:48:59]

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

[commit-bot: I haz the power, 2017-03-14 11:49:00]

Dry run: Try jobs failed on following builders:
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/18543)

[nisse-webrtc, 2017-03-14 11:49:26]

Description was changed from

==========
Delete support for the RTCP RPSI message.

This code has been unused for years, and at least the RTCP sending
logic appears broken.

BUG=webrtc:7338
==========

to

==========
Delete support for the RTCP RPSI and SLI message types.

This code has been unused for years, and at least the RTCP RSPI sending
logic appears broken.

BUG=webrtc:7338
==========

[danilchap, 2017-03-14 12:00:25]

Since deletion turn out to be large, may be better split it into several steps.
red bots and some new todo comments suggest this deletion is not that trivial.

[nisse-webrtc, 2017-03-14 12:03:39]

On 2017/03/14 12:00:25, danilchap wrote:
> Since deletion turn out to be large, may be better split it into several
steps.
> red bots and some new todo comments suggest this deletion is not that trivial.

Red bots were a patch conflict with Tommi's cl
https://codereview.webrtc.org/2744013002

If I split it up, what should be the first piece?

[danilchap, 2017-03-14 12:10:49]

On 2017/03/14 12:03:39, nisse-webrtc wrote:
> On 2017/03/14 12:00:25, danilchap wrote:
> > Since deletion turn out to be large, may be better split it into several
> steps.
> > red bots and some new todo comments suggest this deletion is not that
trivial.
> 
> Red bots were a patch conflict with Tommi's cl
> https://codereview.webrtc.org/2744013002
> 
> If I split it up, what should be the first piece?

I would suggest removing sending rpsi and sli as 1st (two?) cl(s)
2nd step - remove receiving (in rtcp_receiver) of them.

[nisse-webrtc, 2017-03-14 16:12:51]

On 2017/03/14 12:10:49, danilchap wrote:
> On 2017/03/14 12:03:39, nisse-webrtc wrote:
> > On 2017/03/14 12:00:25, danilchap wrote:
> > > Since deletion turn out to be large, may be better split it into several
> > steps.
> > > red bots and some new todo comments suggest this deletion is not that
> trivial.
> > 
> > Red bots were a patch conflict with Tommi's cl
> > https://codereview.webrtc.org/2744013002
> > 
> > If I split it up, what should be the first piece?
> 
> I would suggest removing sending rpsi and sli as 1st (two?) cl(s)
> 2nd step - remove receiving (in rtcp_receiver) of them.

First step now landed. Step two could be the removal of VP8 "feedbackModeOn",
followed 
by purge of RTCP receive code in step three.

[nisse-webrtc, 2017-03-21 09:25:47]

On 2017/03/14 16:12:51, nisse-webrtc wrote:
> On 2017/03/14 12:10:49, danilchap wrote:
> > On 2017/03/14 12:03:39, nisse-webrtc wrote:
> > > On 2017/03/14 12:00:25, danilchap wrote:
> > > > Since deletion turn out to be large, may be better split it into several
> > > steps.
> > > > red bots and some new todo comments suggest this deletion is not that
> > trivial.
> > > 
> > > Red bots were a patch conflict with Tommi's cl
> > > https://codereview.webrtc.org/2744013002
> > > 
> > > If I split it up, what should be the first piece?
> > 
> > I would suggest removing sending rpsi and sli as 1st (two?) cl(s)
> > 2nd step - remove receiving (in rtcp_receiver) of them.
> 
> First step now landed. Step two could be the removal of VP8 "feedbackModeOn",
> followed 
> by purge of RTCP receive code in step three.

Step two, deleting feedbackModeOn, now landed, see cl
https://codereview.webrtc.org/2753783002.

[nisse-webrtc, 2017-03-21 14:38:50]

Description was changed from

==========
Delete support for the RTCP RPSI and SLI message types.

This code has been unused for years, and at least the RTCP RSPI sending
logic appears broken.

BUG=webrtc:7338
==========

to

==========
Delete support for receiving RTCP RPSI and SLI message.

This code has been unused for years, and at least the RTCP RSPI sending
logic appears broken.

This cl is part 3, following 

  https://codereview.webrtc.org/2746413003 (delete sending)
  https://codereview.webrtc.org/2753783002 (delete vp8 feedback mode)

BUG=webrtc:7338
==========

[nisse-webrtc, 2017-03-21 14:39:36]

Hi, do you think you can review the remaining delete in one piece?

[danilchap, 2017-03-21 14:53:25]

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/common_types.h#ne...
webrtc/common_types.h:465: bool pictureLossIndicationOn;
anything hold you from deleting this field too?

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h:99: // kRtcpRpsi, but is no
longer used. Renumber?
do not renumber, there is enough bits to leave the gap.
It would make more sense to resolve sprang's todo above (then it would be
renumbered completely or disappear)

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h:226: // updated.
intead of TODO add RTC_DEPRECATED to ensure no webrtc code uses it and  other
projects are warned

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h:230: // TODO(nisse):
Obsolete, delete as soon as applications are
ditto

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_utility.cc (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_utility.cc:111: * FMT 2:     Slice Lost
Indication (SLI)
either add 'not implemented' this line too. or do not add 'not implemented' next
line (preferred since other 'non implemented' messages do not have that note)

[nisse-webrtc, 2017-03-21 15:12:55]

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/common_types.h#ne...
webrtc/common_types.h:465: bool pictureLossIndicationOn;
On 2017/03/21 14:53:24, danilchap wrote:
> anything hold you from deleting this field too?

There are some tests for it, and I haven't researched references outside of
webrtc. So I'd prefer to delete it in a separate cl.

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h:99: // kRtcpRpsi, but is no
longer used. Renumber?
On 2017/03/21 14:53:24, danilchap wrote:
> do not renumber, there is enough bits to leave the gap.
> It would make more sense to resolve sprang's todo above (then it would be
> renumbered completely or disappear)

Ok, I deleted this comment (there was already an unexplained gap, 0x2000).

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h:226: // updated.
On 2017/03/21 14:53:24, danilchap wrote:
> intead of TODO add RTC_DEPRECATED to ensure no webrtc code uses it and  other
> projects are warned

I'll give it a try, but if downstream projects are compiled with 
-Werror RTC_DEPRECATED will break them as badly as just deleting the method.

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_utility.cc (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_utility.cc:111: * FMT 2:     Slice Lost
Indication (SLI)
On 2017/03/21 14:53:24, danilchap wrote:
> either add 'not implemented' this line too. or do not add 'not implemented'
next
> line (preferred since other 'non implemented' messages do not have that note)

Delete the note.

[danilchap, 2017-03-21 15:19:42]

I'm not familiar enough with code in vp8_impl.h/vp8_impl.cc
Everything else lgtm

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/2742383004/diff/180001/webrtc/common_types.h#ne...
webrtc/common_types.h:465: bool pictureLossIndicationOn;
On 2017/03/21 15:12:55, nisse-webrtc wrote:
> On 2017/03/21 14:53:24, danilchap wrote:
> > anything hold you from deleting this field too?
> 
> There are some tests for it, and I haven't researched references outside of
> webrtc. So I'd prefer to delete it in a separate cl.

Ok

[stefan-webrtc, 2017-03-22 13:01:23]

lgtm

[nisse-webrtc, 2017-03-22 13:05:56]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-03-22 13:05:57]

The patchset sent to the CQ was uploaded after l-g-t-m from danilchap@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2742383004/#ps220001 (title:
"Rebased.")

[commit-bot: I haz the power, 2017-03-22 13:06:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-22 13:32:31]

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

[commit-bot: I haz the power, 2017-03-22 13:32:32]

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

[nisse-webrtc, 2017-03-22 13:58:13]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2017-03-22 13:58:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-22 14:15:10]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1490191093224090,
"parent_rev": "b69aec59f38db501f4b967eb70e21854e5ef9978", "commit_rev":
"25d0bdc1bcbd78adabe5dac4ff965434cd83a41f"}

[commit-bot: I haz the power, 2017-03-22 14:15:13]

Description was changed from

==========
Delete support for receiving RTCP RPSI and SLI message.

This code has been unused for years, and at least the RTCP RSPI sending
logic appears broken.

This cl is part 3, following 

  https://codereview.webrtc.org/2746413003 (delete sending)
  https://codereview.webrtc.org/2753783002 (delete vp8 feedback mode)

BUG=webrtc:7338
==========

to

==========
Delete support for receiving RTCP RPSI and SLI message.

This code has been unused for years, and at least the RTCP RSPI sending
logic appears broken.

This cl is part 3, following 

  https://codereview.webrtc.org/2746413003 (delete sending)
  https://codereview.webrtc.org/2753783002 (delete vp8 feedback mode)

BUG=webrtc:7338

Review-Url: https://codereview.webrtc.org/2742383004
Cr-Commit-Position: refs/heads/master@{#17342}
Committed:
https://chromium.googlesource.com/external/webrtc/+/25d0bdc1bcbd78adabe5dac4f...
==========

[commit-bot: I haz the power, 2017-03-22 14:15:15]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/external/webrtc/+/25d0bdc1bcbd78adabe5dac4f...