RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up

RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=webrtc:5805
Committed: https://crrev.com/425a6ccac3c6c485048685566b259c540280cc2a
Cr-Commit-Position: refs/heads/master@{#14530}

[ossu, 2016-10-03 11:45:09]

ossu@webrtc.org changed reviewers:
+ danilchap@webrtc.org, henrik.lundin@webrtc.org

[ossu, 2016-10-03 11:45:10]

This CL simplifies (well, removes) some of the code in RTPReceiverAudio, to get
rid of G722 specific code around WebRTC.

[danilchap, 2016-10-03 14:29:38]

lgtm,
nice to see dead code removed!

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:174: void
RTPReceiverAudio::CheckPayloadChanged(int8_t payload_type,
may be pull string further and change signature of this function into something
like
bool RTPReceiverStrategy::ShouldDiscardChanges(int payload_type);

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h:44: // Returns true if CNG
is configured with payload type payload_type.
may be just
...configured with |payload_type|.

[hlundin-webrtc, 2016-10-04 11:38:18]

lgtm with comments.

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc (left):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:160: //  we can have three
CNG on 8000Hz, 16000Hz and 32000Hz
lol

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:62: //  We can have four
CNG on 8000Hz, 16000Hz, 32000Hz and 48000Hz.
Wording is a bit choppy in this comment. Bonus points if you improve that too.

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:174: void
RTPReceiverAudio::CheckPayloadChanged(int8_t payload_type,
On 2016/10/03 14:29:38, danilchap wrote:
> may be pull string further and change signature of this function into
something
> like
> bool RTPReceiverStrategy::ShouldDiscardChanges(int payload_type);

Would be nice, but I'm afraid that will lead into RTPReceiverVideo as well.

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:175: PayloadUnion*
specific_payload,
/* specific_payload */

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h:44: // Returns true if CNG
is configured with payload type payload_type.
On 2016/10/03 14:29:38, danilchap wrote:
> may be just
> ...configured with |payload_type|.

+1

[ossu, 2016-10-04 12:40:24]

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:62: //  We can have four
CNG on 8000Hz, 16000Hz, 32000Hz and 48000Hz.
On 2016/10/04 11:38:18, hlundin-webrtc wrote:
> Wording is a bit choppy in this comment. Bonus points if you improve that too.

I've reworded here and moved it down to where the other similar comment was. It
makes more sense to clarify there than here.

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:175: PayloadUnion*
specific_payload,
On 2016/10/04 11:38:18, hlundin-webrtc wrote:
> /* specific_payload */

Done.

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h:44: // Returns true if CNG
is configured with payload type payload_type.
On 2016/10/04 11:38:18, hlundin-webrtc wrote:
> On 2016/10/03 14:29:38, danilchap wrote:
> > may be just
> > ...configured with |payload_type|.
> 
> +1

Alright, alright, calm down! :)

[danilchap, 2016-10-04 12:45:04]

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h:44: // Returns true if CNG
is configured with payload type payload_type.
On 2016/10/04 12:40:24, ossu wrote:
> On 2016/10/04 11:38:18, hlundin-webrtc wrote:
> > On 2016/10/03 14:29:38, danilchap wrote:
> > > may be just
> > > ...configured with |payload_type|.
> > 
> > +1
> 
> Alright, alright, calm down! :)

May be change this (#44) line too? :)

[ossu, 2016-10-04 12:49:15]

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h (right):

https://codereview.webrtc.org/2383103002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.h:44: // Returns true if CNG
is configured with payload type payload_type.
On 2016/10/04 12:45:04, danilchap wrote:
> On 2016/10/04 12:40:24, ossu wrote:
> > On 2016/10/04 11:38:18, hlundin-webrtc wrote:
> > > On 2016/10/03 14:29:38, danilchap wrote:
> > > > may be just
> > > > ...configured with |payload_type|.
> > > 
> > > +1
> > 
> > Alright, alright, calm down! :)
> 
> May be change this (#44) line too? :)

GAH!

[ossu, 2016-10-04 14:53:40]

Description was changed from

==========
RTPReceiverAudio: Removed frequency fron CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=
==========

to

==========
RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=webrtc:5805
==========

[ossu, 2016-10-05 11:49:17]

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

[commit-bot: I haz the power, 2016-10-05 11:49:26]

Dry run: 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, 2016-10-05 12:18:07]

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

[commit-bot: I haz the power, 2016-10-05 12:18:08]

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

[ossu, 2016-10-05 15:32:44]

The CQ bit was checked by ossu@webrtc.org

[ossu, 2016-10-05 15:32:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, danilchap@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2383103002/#ps60001 (title:
"Rebase")

[commit-bot: I haz the power, 2016-10-05 15:32:48]

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, 2016-10-05 15:44:24]

Description was changed from

==========
RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=webrtc:5805
==========

to

==========
RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=webrtc:5805
==========

[commit-bot: I haz the power, 2016-10-05 15:44:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-05 15:44:34]

Description was changed from

==========
RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=webrtc:5805
==========

to

==========
RTPReceiverAudio: Removed frequency from CNGPayloadType and cleaned up
CheckPayloadChanged.

Removed last_received_frequency_, cng_payload_type_,
g722_payload_type_ and last_received_g722_ from RTPReceiverAudio and
cleaned up most of the related, now dead code.

Since g722_payload_type_ was never set, neither was
last_received_g722_, which means the frequency change in
CNGPayloadType was never done. Setting the frequency to the standard
values also proved unnecessary, since they were already set before the
call. Even if frequency would have been changed by RTPReceiverAudio, I
was not able to find a place where that would actually have
mattered. The ACM and NetEq, for example, which eventually gets these
packages, don't care about that value.

Also, GetPayloadTypeFrequency was never called, so keeping track of
last_received_frequency_ proved unnecessary.

cng_payload_type_ was stored to be able to check in CNGPayloadType if
cng_payload_type_has_changed. This flag was also never read, so these
all disappear.

The main reason for starting this change was to root out any G722
specific code we have sprinkled around the code base (specifically
dealing with the fact that for G722 clock rate != sample rate). In
this case, once I started pulling at one end of the string, the whole
thing came unraveled.

BUG=webrtc:5805

Committed: https://crrev.com/425a6ccac3c6c485048685566b259c540280cc2a
Cr-Commit-Position: refs/heads/master@{#14530}
==========

[commit-bot: I haz the power, 2016-10-05 15:44:35]

Patchset 4 (id:??) landed as
https://crrev.com/425a6ccac3c6c485048685566b259c540280cc2a
Cr-Commit-Position: refs/heads/master@{#14530}