Remove webrtc/stream.h and unutilized inheritance.

Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
TBR=mflodman@webrtc.org

Committed: https://crrev.com/1ba8d39a9cee4d8771faa440ac4d2978170fc8ea
Cr-Commit-Position: refs/heads/master@{#12586}

[pbos-webrtc, 2016-04-27 14:15:59]

Description was changed from

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=solenberg@webrtc.org
==========

to

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
==========

[pbos-webrtc, 2016-04-27 14:15:59]

pbos@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[pbos-webrtc, 2016-04-27 14:16:23]

PTAL, I'd like to get rid of a root dependency and some inheritance that we
weren't really using.

[pbos-webrtc, 2016-04-27 14:17:11]

PTAL, I'd like to get rid of a root dependency and some inheritance that we
weren't really using.

[kjellander_webrtc, 2016-04-27 14:23:31]

One down! lgtm

[The Sun (google.com), 2016-04-27 19:39:45]

solenberg@google.com changed reviewers:
+ solenberg@google.com

[The Sun (google.com), 2016-04-27 19:39:47]

Good stuff! I believe you can make it even better.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_receive_st...
File webrtc/audio/audio_receive_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_receive_st...
webrtc/audio/audio_receive_stream.h:39: void SignalNetworkState(NetworkState
state);
nit: move this to sit together with config()
another nit: also update .cc to have the same order as the methods here.

...and likewise for AudioSendStream etc.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_send_stream.h
File webrtc/audio/audio_send_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_send_strea...
webrtc/audio/audio_send_stream.h:39: // webrtc::SendStream implementation.
nit: comment is wrong now. order methods as in the base class, here and in .cc

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_receive_stream.h
File webrtc/audio_receive_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_receive_stream.h...
webrtc/audio_receive_stream.h:114: virtual bool DeliverRtcp(const uint8_t*
packet, size_t length) = 0;
These don't even belong here as a virtual methods. It could be a non-virtual on
the impl class since packets are always delivered through i.e.
call_->Receiver()->DeliverPacket(...)

The client never pushes packets directly to the stream.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_send_stream.h
File webrtc/audio_send_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_send_stream.h#ne...
webrtc/audio_send_stream.h:98: virtual bool DeliverRtcp(const uint8_t* packet,
size_t length) = 0;
same as in AudioReceiveStream - only keep this in impl class. This is the client
interface.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
File webrtc/media/engine/fakewebrtccall.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
webrtc/media/engine/fakewebrtccall.h:53: bool DeliverRtcp(const uint8_t* packet,
size_t length) override {
Unused, remove.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
webrtc/media/engine/fakewebrtccall.h:82: bool DeliverRtcp(const uint8_t* packet,
size_t length) override {
etc

https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
webrtc/media/engine/fakewebrtccall.h:128: bool DeliverRtcp(const uint8_t*
packet, size_t length) override {
etc

https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
webrtc/media/engine/fakewebrtccall.h:167: bool DeliverRtcp(const uint8_t*
packet, size_t length) override {
etc

https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_receive_stream.h
File webrtc/video_receive_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_receive_stream.h...
webrtc/video_receive_stream.h:186: // Deliver an incoming RTP packet.
No need for these in the client interface, like for audio.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_send_stream.h
File webrtc/video_send_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_send_stream.h#ne...
webrtc/video_send_stream.h:176: virtual bool DeliverRtcp(const uint8_t* packet,
size_t length) = 0;
No need to  have this in client interface, like for audio.

[The Sun (google.com), 2016-04-27 19:42:17]

On 2016/04/27 19:39:47, The Sun (google.com) wrote:
> Good stuff! I believe you can make it even better.
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_receive_st...
> File webrtc/audio/audio_receive_stream.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_receive_st...
> webrtc/audio/audio_receive_stream.h:39: void SignalNetworkState(NetworkState
> state);
> nit: move this to sit together with config()
> another nit: also update .cc to have the same order as the methods here.
> 
> ...and likewise for AudioSendStream etc.
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_send_stream.h
> File webrtc/audio/audio_send_stream.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_send_strea...
> webrtc/audio/audio_send_stream.h:39: // webrtc::SendStream implementation.
> nit: comment is wrong now. order methods as in the base class, here and in .cc
> 
> https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_receive_stream.h
> File webrtc/audio_receive_stream.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_receive_stream.h...
> webrtc/audio_receive_stream.h:114: virtual bool DeliverRtcp(const uint8_t*
> packet, size_t length) = 0;
> These don't even belong here as a virtual methods. It could be a non-virtual
on
> the impl class since packets are always delivered through i.e.
> call_->Receiver()->DeliverPacket(...)
> 
> The client never pushes packets directly to the stream.
> 
> https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_send_stream.h
> File webrtc/audio_send_stream.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_send_stream.h#ne...
> webrtc/audio_send_stream.h:98: virtual bool DeliverRtcp(const uint8_t* packet,
> size_t length) = 0;
> same as in AudioReceiveStream - only keep this in impl class. This is the
client
> interface.
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
> File webrtc/media/engine/fakewebrtccall.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
> webrtc/media/engine/fakewebrtccall.h:53: bool DeliverRtcp(const uint8_t*
packet,
> size_t length) override {
> Unused, remove.
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
> webrtc/media/engine/fakewebrtccall.h:82: bool DeliverRtcp(const uint8_t*
packet,
> size_t length) override {
> etc
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
> webrtc/media/engine/fakewebrtccall.h:128: bool DeliverRtcp(const uint8_t*
> packet, size_t length) override {
> etc
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/media/engine/fakewebrt...
> webrtc/media/engine/fakewebrtccall.h:167: bool DeliverRtcp(const uint8_t*
> packet, size_t length) override {
> etc
> 
> https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_receive_stream.h
> File webrtc/video_receive_stream.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_receive_stream.h...
> webrtc/video_receive_stream.h:186: // Deliver an incoming RTP packet.
> No need for these in the client interface, like for audio.
> 
> https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_send_stream.h
> File webrtc/video_send_stream.h (right):
> 
>
https://codereview.webrtc.org/1924793002/diff/1/webrtc/video_send_stream.h#ne...
> webrtc/video_send_stream.h:176: virtual bool DeliverRtcp(const uint8_t*
packet,
> size_t length) = 0;
> No need to  have this in client interface, like for audio.

(sorry, used the wrong persona)

[The Sun (google.com), 2016-04-27 19:42:29]

solenberg@google.com changed reviewers:
- solenberg@google.com

[pbos-webrtc, 2016-04-28 07:10:13]

PTAL

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_receive_st...
File webrtc/audio/audio_receive_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_receive_st...
webrtc/audio/audio_receive_stream.h:39: void SignalNetworkState(NetworkState
state);
On 2016/04/27 19:39:46, The Sun (google.com) wrote:
> nit: move this to sit together with config()
> another nit: also update .cc to have the same order as the methods here.
> 
> ...and likewise for AudioSendStream etc.

Done.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_send_stream.h
File webrtc/audio/audio_send_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio/audio_send_strea...
webrtc/audio/audio_send_stream.h:39: // webrtc::SendStream implementation.
On 2016/04/27 19:39:46, The Sun (google.com) wrote:
> nit: comment is wrong now. order methods as in the base class, here and in .cc

Done.

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_receive_stream.h
File webrtc/audio_receive_stream.h (right):

https://codereview.webrtc.org/1924793002/diff/1/webrtc/audio_receive_stream.h...
webrtc/audio_receive_stream.h:114: virtual bool DeliverRtcp(const uint8_t*
packet, size_t length) = 0;
On 2016/04/27 19:39:46, The Sun (google.com) wrote:
> These don't even belong here as a virtual methods. It could be a non-virtual
on
> the impl class since packets are always delivered through i.e.
> call_->Receiver()->DeliverPacket(...)
> 
> The client never pushes packets directly to the stream.

Done.

[pbos-webrtc, 2016-04-28 07:10:18]

solenberg@ feedback

[the sun, 2016-04-28 08:16:22]

lgtm, but consider landing after mflodman@'s CL
https://codereview.webrtc.org/1909333002/ as I suspect his merge would be more
intricate

[pbos-webrtc, 2016-05-02 01:57:43]

rebase

[pbos-webrtc, 2016-05-02 02:04:19]

re-rebase

[pbos-webrtc, 2016-05-02 02:06:45]

The CQ bit was checked by pbos@webrtc.org

[pbos-webrtc, 2016-05-02 02:06:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org, solenberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1924793002/#ps60001 (title:
"re-rebase")

[commit-bot: I haz the power, 2016-05-02 02:06:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1924793002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1924793002/60001

[pbos-webrtc, 2016-05-02 02:18:38]

Description was changed from

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
==========

to

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
TBR=mflodman@webrtc.org
==========

[pbos-webrtc, 2016-05-02 02:19:05]

pbos@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[pbos-webrtc, 2016-05-02 02:19:06]

TBR=mflodman@ for the root-header rubberstamp.

[commit-bot: I haz the power, 2016-05-02 03:18:36]

Description was changed from

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
TBR=mflodman@webrtc.org
==========

to

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
TBR=mflodman@webrtc.org
==========

[commit-bot: I haz the power, 2016-05-02 03:18:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-02 03:18:42]

Description was changed from

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
TBR=mflodman@webrtc.org
==========

to

==========
Remove webrtc/stream.h and unutilized inheritance.

Removes inheritance and a virtual call. Also removes a root header that
would have needed to be moved into a subdirectory otherwise to prevent
circular dependencies.

BUG=webrtc:4243
R=kjellander@webrtc.org, solenberg@webrtc.org
TBR=mflodman@webrtc.org

Committed: https://crrev.com/1ba8d39a9cee4d8771faa440ac4d2978170fc8ea
Cr-Commit-Position: refs/heads/master@{#12586}
==========

[commit-bot: I haz the power, 2016-05-02 03:18:44]

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