Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=webrtc:5806
Committed: https://crrev.com/00bceb1eda186848a7883b43d024d42a67d724b5
Cr-Commit-Position: refs/heads/master@{#15380}

[ossu, 2016-12-01 13:58:48]

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

[commit-bot: I haz the power, 2016-12-01 13:58:50]

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

[ossu, 2016-12-01 13:59:12]

Description was changed from

==========
Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=wbrtc:5806
==========

to

==========
Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=webrtc:5806
==========

[ossu, 2016-12-01 14:09:02]

ossu@webrtc.org changed reviewers:
+ hlundin@google.com, solenberg@webrtc.org

[ossu, 2016-12-01 14:09:03]

Paving a bit of the road towards injectable audio encoders.
PTAL!

[hlundin-webrtc, 2016-12-01 14:13:31]

henrik.lundin@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[hlundin-webrtc, 2016-12-01 14:13:32]

lgtm with one nit.

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

https://codereview.webrtc.org/2545753002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc:132: const int
kDtmfIntervalTimeMs = 50;
constexpr?

[commit-bot: I haz the power, 2016-12-01 14:33:44]

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

[commit-bot: I haz the power, 2016-12-01 14:33:45]

Dry run: This issue passed the CQ dry run.

[the sun, 2016-12-01 19:43:08]

lgtm

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

https://codereview.webrtc.org/2545753002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc:165: if (rtp_timestamp -
dtmf_timestamp_last_sent_ < dtmf_interval_time_rtp) {
nit: add parenthesis around the difference - make the code easier to read imo

[ossu, 2016-12-02 10:24:42]

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

https://codereview.webrtc.org/2545753002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc:132: const int
kDtmfIntervalTimeMs = 50;
On 2016/12/01 14:13:32, hlundin-webrtc wrote:
> constexpr?

Done.

https://codereview.webrtc.org/2545753002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc:165: if (rtp_timestamp -
dtmf_timestamp_last_sent_ < dtmf_interval_time_rtp) {
On 2016/12/01 19:43:08, the sun wrote:
> nit: add parenthesis around the difference - make the code easier to read imo

Done.

[ossu, 2016-12-02 10:24:47]

The CQ bit was checked by ossu@webrtc.org

[ossu, 2016-12-02 10:24:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, henrik.lundin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2545753002/#ps20001 (title:
"constexpr; parentheses")

[commit-bot: I haz the power, 2016-12-02 10:24:49]

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-12-02 10:40:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480674287153220,
"parent_rev": "e5449767d147e107b7f781fa5adb05b2e468a5d9", "commit_rev":
"ea61f6814f4d1d03932d264bfd10f2a6df8dff37"}

[commit-bot: I haz the power, 2016-12-02 10:40:06]

Description was changed from

==========
Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=webrtc:5806
==========

to

==========
Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=webrtc:5806
==========

[commit-bot: I haz the power, 2016-12-02 10:40:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-02 10:40:14]

Description was changed from

==========
Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=webrtc:5806
==========

to

==========
Deprecated SetAudioPacketSize from RTPSender and removed calls to it.

The packet size was only used to control how often to output DTMF
packets. However, it likely did not work as intended, since that
interval was only set during initialization. No changes to the packet
size, like what AudioEncoder::Num10MsFramesInNextPacket could
indicate, were picked up. The value was instead taken from an entry in
ACMCodecDB.

Since it was not-fully-functional, its exact value didn't seem to
matter and it was getting in the way of making it possible to supply
an external audio encoder factory, I've decided to remove it
altogether. The DTMF code now uses an interval of 50 ms regardless,
which is a value recommended by the RFC.

BUG=webrtc:5806

Committed: https://crrev.com/00bceb1eda186848a7883b43d024d42a67d724b5
Cr-Commit-Position: refs/heads/master@{#15380}
==========

[commit-bot: I haz the power, 2016-12-02 10:40:15]

Patchset 2 (id:??) landed as
https://crrev.com/00bceb1eda186848a7883b43d024d42a67d724b5
Cr-Commit-Position: refs/heads/master@{#15380}