Move back receiver VCM into ViEChannel.

Move back receiver VCM into ViEChannel.

Contains callbacks to ViEChannel and must expire before it. There are
currently use-after-frees on ViEChannel or related objects.

BUG=webrtc:5494
R=danilchap@webrtc.org

[pbos-webrtc, 2016-02-15 11:08:22]

Description was changed from

==========
Move back receiver VCM into ViEChannel.

Contains callbacks to ViEChannel and must expire before it. There are
currently use-after-frees on ViEChannel or related objects because of
this.

BUG=webrtc:5494
R=stefan@webrtc.org
==========

to

==========
Move back receiver VCM into ViEChannel.

Contains callbacks to ViEChannel and must expire before it. There are
currently use-after-frees on ViEChannel or related objects because of
this.

BUG=webrtc:5494
R=danilchap@webrtc.org
==========

[pbos-webrtc, 2016-02-15 11:08:22]

pbos@webrtc.org changed reviewers:
+ danilchap@webrtc.org
- stefan@webrtc.org

[pbos-webrtc, 2016-02-15 11:08:28]

PTAL

[pbos-webrtc, 2016-02-15 11:10:45]

Description was changed from

==========
Move back receiver VCM into ViEChannel.

Contains callbacks to ViEChannel and must expire before it. There are
currently use-after-frees on ViEChannel or related objects because of
this.

BUG=webrtc:5494
R=danilchap@webrtc.org
==========

to

==========
Move back receiver VCM into ViEChannel.

Contains callbacks to ViEChannel and must expire before it. There are
currently use-after-frees on ViEChannel or related objects.

BUG=webrtc:5494
R=danilchap@webrtc.org
==========

[danilchap, 2016-02-15 12:25:09]

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
File webrtc/video/video_receive_stream.cc (left):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
webrtc/video/video_receive_stream.cc:380: // TODO(pbos): Wire up
config_.render->IsTextureSupported() and convert if not
is it related to this change or rogue cleanup?

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
webrtc/video/video_receive_stream.cc:304: process_thread_->RegisterModule(vcm_);
Looks strange that a non-owner object configure and register vcm. Can you put
comment why it is done that way?

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (right):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.cc#n...
webrtc/video/vie_channel.cc:164: if (vcm_->RegisterReceiveCallback(this) != 0) {
Can it make sense to join this callbacks initialization with callbacks in
VideoReceiveStream to keep all initialization of vcm_ in one place?

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.h
File webrtc/video/vie_channel.h (right):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.h#ne...
webrtc/video/vie_channel.h:360: const rtc::scoped_ptr<VideoCodingModule> vcm_;
std::unique_ptr instead of rtc::scoped_ptr is now allowed.

[pbos-webrtc, 2016-02-15 13:04:23]

feedback

[pbos-webrtc, 2016-02-15 13:04:24]

PTAL

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
File webrtc/video/video_receive_stream.cc (left):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
webrtc/video/video_receive_stream.cc:380: // TODO(pbos): Wire up
config_.render->IsTextureSupported() and convert if not
On 2016/02/15 12:25:09, danilchap wrote:
> is it related to this change or rogue cleanup?

Rogue, added years ago I think.

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
webrtc/video/video_receive_stream.cc:304: process_thread_->RegisterModule(vcm_);
On 2016/02/15 12:25:09, danilchap wrote:
> Looks strange that a non-owner object configure and register vcm. Can you put
> comment why it is done that way?

Done.

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (right):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.cc#n...
webrtc/video/vie_channel.cc:164: if (vcm_->RegisterReceiveCallback(this) != 0) {
On 2016/02/15 12:25:09, danilchap wrote:
> Can it make sense to join this callbacks initialization with callbacks in
> VideoReceiveStream to keep all initialization of vcm_ in one place?

Done.

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.h
File webrtc/video/vie_channel.h (right):

https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.h#ne...
webrtc/video/vie_channel.h:360: const rtc::scoped_ptr<VideoCodingModule> vcm_;
On 2016/02/15 12:25:09, danilchap wrote:
> std::unique_ptr instead of rtc::scoped_ptr is now allowed.

Will replace them all when they've been used more elsewhere, so keeping
scoped_ptr for consistency.

[pbos-webrtc, 2016-02-15 16:53:37]

On 2016/02/15 13:04:24, pbos-webrtc wrote:
> PTAL
> 
>
https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
> File webrtc/video/video_receive_stream.cc (left):
> 
>
https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
> webrtc/video/video_receive_stream.cc:380: // TODO(pbos): Wire up
> config_.render->IsTextureSupported() and convert if not
> On 2016/02/15 12:25:09, danilchap wrote:
> > is it related to this change or rogue cleanup?
> 
> Rogue, added years ago I think.
> 
>
https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
> File webrtc/video/video_receive_stream.cc (right):
> 
>
https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/video_receive_st...
> webrtc/video/video_receive_stream.cc:304:
process_thread_->RegisterModule(vcm_);
> On 2016/02/15 12:25:09, danilchap wrote:
> > Looks strange that a non-owner object configure and register vcm. Can you
put
> > comment why it is done that way?
> 
> Done.
> 
> https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.cc
> File webrtc/video/vie_channel.cc (right):
> 
>
https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.cc#n...
> webrtc/video/vie_channel.cc:164: if (vcm_->RegisterReceiveCallback(this) != 0)
{
> On 2016/02/15 12:25:09, danilchap wrote:
> > Can it make sense to join this callbacks initialization with callbacks in
> > VideoReceiveStream to keep all initialization of vcm_ in one place?
> 
> Done.
> 
> https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.h
> File webrtc/video/vie_channel.h (right):
> 
>
https://codereview.webrtc.org/1701683002/diff/1/webrtc/video/vie_channel.h#ne...
> webrtc/video/vie_channel.h:360: const rtc::scoped_ptr<VideoCodingModule> vcm_;
> On 2016/02/15 12:25:09, danilchap wrote:
> > std::unique_ptr instead of rtc::scoped_ptr is now allowed.
> 
> Will replace them all when they've been used more elsewhere, so keeping
> scoped_ptr for consistency.

Submitted https://codereview.webrtc.org/1699893002/ instead.