Reland of Split IncomingVideoStream into two implementations, with smoothing and without.

Reland of Split IncomingVideoStream into two implementations, with smoothing and without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were delivered on the decoder thread.  No-smoothing is now done in a separate class that uses a TaskQueue.  The implementation may drop frames if the renderer doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.  Now, when an instance is created, it should be considered to be "running" and when it is deleted, it's "not running".  This saves on resources and also reduces the amount of locking required and I could remove one critical section altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This means that any implementation of that interface can be used and the decoder can be made to  just use the 'renderer' from the config.  Once we do that, we can decouple the IncomingVideoStream implementations from the decoder and VideoReceiveStream implementations and leave it up to the application for how to do smoothing.  The app can choose to use the Incoming* classes or roll its own (which may be preferable since applications often have their own scheduling mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1 outstanding pending frame.  If we exceed that, the current frame won't be delivered to the renderer and instead we deliver the next one (since when this happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream classes are also constructed/destructed at the same time as the owning object, which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the VideoReceiveStream render delay with a fixed value of 10ms on construction.  This wasn't a problem with the previous implementation (it would be now though) but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
TBR=mflodman

Committed: https://crrev.com/e03f8787377bbc03a4e00184bb14b7561b108cbb
Cr-Commit-Position: refs/heads/master@{#13175}

[tommi, 2016-06-15 13:46:48]

Created Reland of Split IncomingVideoStream into two implementations, with
smoothing and without.

[tommi, 2016-06-15 13:48:16]

Description was changed from

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without. (patchset #1 id:1 of https://codereview.webrtc.org/2061363002/ )

Reason for revert:
Prepare for a reland.  Seems to be fine in Chrome and other Windows bots.

Original issue's description:
> Revert of Split IncomingVideoStream into two implementations, with smoothing
and without. (patchset #23 id:430001 of
https://codereview.webrtc.org/2035173002/ )
> 
> Reason for revert:
> Reverting while we track down the issue on the Win10 bot.
> 
> Original issue's description:
> > Split IncomingVideoStream into two implementations, with smoothing and
without.
> >
> > This CL fixes an issue with the non-smoothing implementation where frames
were delivered on the decoder thread.  No-smoothing is now done in a separate
class that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.
> >
> > Further work done:
> >
> > * I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.
> >
> > * I removed the Start/Stop methods from the IncomingVideoStream
implementations.  Now, when an instance is created, it should be considered to
be "running" and when it is deleted, it's "not running".  This saves on
resources and also reduces the amount of locking required and I could remove one
critical section altogether.
> >
> > * I changed the VideoStreamDecoder class to not depend on
IncomingVideoStream but rather use the generic
rtc::VideoSinkInterface<VideoFrame> interface.  This means that any
implementation of that interface can be used and the decoder can be made to 
just use the 'renderer' from the config.  Once we do that, we can decouple the
IncomingVideoStream implementations from the decoder and VideoReceiveStream
implementations and leave it up to the application for how to do smoothing.  The
app can choose to use the Incoming* classes or roll its own (which may be
preferable since applications often have their own scheduling mechanisms).
> >
> > * The non-smoothing IncomingVideoStream implementation currently allows only
1 outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).
> >
> > * The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop
in VideoReceiveStream and not all of the lifetime of VideoReceiveStream.
> >
> > * Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)
> >
> > * Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.
> >
> > * Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.
> >
> > * Made the render delay value in VideoRenderFrames, const.
> >
> > BUG=
> >
> > Committed: https://crrev.com/1c7eef652b0aa22d8ebb0bfe2b547094a794be22
> > Cr-Commit-Position: refs/heads/master@{#13129}
> 
> TBR=mflodman@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=
> 
> Committed: https://crrev.com/17c3cddf9d44874f198c99097313c9b76e33ba05
> Cr-Commit-Position: refs/heads/master@{#13146}

TBR=mflodman@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
==========

to

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

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

[tommi, 2016-06-15 14:04:19]

Remove frame dropping logic in the non-smoothing case

[tommi, 2016-06-15 14:13:14]

Please take a look Magnus.  I had intended for the 2nd patch set to show the
differences in these files:

* incoming_video_stream.h
* incoming_video_stream.cc
* incoming_video_stream_unittest.cc

Which is where there are changes (basically deleting the queueing code and
updating a test accordingly).  Unfortunately patching locally and uploading,
also included rebasing, so all files have something in them, but the above ones
are the only relevant changes to this cl.

[tommi, 2016-06-15 14:14:56]

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

[commit-bot: I haz the power, 2016-06-15 14:15:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2071473002/250001

[commit-bot: I haz the power, 2016-06-15 14:25:16]

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

[commit-bot: I haz the power, 2016-06-15 14:25:17]

Dry run: Exceeded global retry quota

[tommi, 2016-06-15 14:53:25]

Description was changed from

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

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

to

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
==========

[tommi, 2016-06-15 15:03:02]

Fix check in test

[tommi, 2016-06-15 15:03:13]

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

[commit-bot: I haz the power, 2016-06-15 15:03:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2071473002/270001

[commit-bot: I haz the power, 2016-06-15 15:08:05]

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

[commit-bot: I haz the power, 2016-06-15 15:08:06]

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

[tommi, 2016-06-16 15:40:02]

Description was changed from

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
==========

to

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
TBR=mflodman
==========

[tommi, 2016-06-16 15:40:38]

tbr-ing since the only change is to remove the drop logic for non-smoothing.

[tommi, 2016-06-16 15:40:43]

The CQ bit was checked by tommi@webrtc.org

[commit-bot: I haz the power, 2016-06-16 15:40:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2071473002/270001

[commit-bot: I haz the power, 2016-06-16 15:47:17]

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

[commit-bot: I haz the power, 2016-06-16 15:47:19]

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

[tommi, 2016-06-16 15:55:16]

add explicit

[tommi, 2016-06-16 15:55:41]

The CQ bit was checked by tommi@webrtc.org

[commit-bot: I haz the power, 2016-06-16 15:55:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2071473002/290001

[commit-bot: I haz the power, 2016-06-16 16:05:44]

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

[commit-bot: I haz the power, 2016-06-16 16:05:45]

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

[tommi, 2016-06-16 20:01:39]

The CQ bit was checked by tommi@webrtc.org

[commit-bot: I haz the power, 2016-06-16 20:01:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2071473002/290001

[commit-bot: I haz the power, 2016-06-16 20:29:06]

Description was changed from

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
TBR=mflodman
==========

to

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
TBR=mflodman
==========

[commit-bot: I haz the power, 2016-06-16 20:29:08]

Committed patchset #4 (id:290001)

[commit-bot: I haz the power, 2016-06-16 20:29:20]

Description was changed from

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
TBR=mflodman
==========

to

==========
Reland of Split IncomingVideoStream into two implementations, with smoothing and
without.

Original issue's description:

Split IncomingVideoStream into two implementations, with smoothing and without.

This CL fixes an issue with the non-smoothing implementation where frames were
delivered on the decoder thread.  No-smoothing is now done in a separate class
that uses a TaskQueue.  The implementation may drop frames if the renderer
doesn't keep up and it doesn't block the decoder thread.

Further work done:

* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we
today grab 5 locks.

* I removed the Start/Stop methods from the IncomingVideoStream implementations.
 Now, when an instance is created, it should be considered to be "running" and
when it is deleted, it's "not running".  This saves on resources and also
reduces the amount of locking required and I could remove one critical section
altogether.

* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream
but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface.  This
means that any implementation of that interface can be used and the decoder can
be made to  just use the 'renderer' from the config.  Once we do that, we can
decouple the IncomingVideoStream implementations from the decoder and
VideoReceiveStream implementations and leave it up to the application for how to
do smoothing.  The app can choose to use the Incoming* classes or roll its own
(which may be preferable since applications often have their own scheduling
mechanisms).

* The non-smoothing IncomingVideoStream implementation currently allows only 1
outstanding pending frame.  If we exceed that, the current frame won't be
delivered to the renderer and instead we deliver the next one (since when this
happens, the renderer is falling behind).

* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in
VideoReceiveStream and not all of the lifetime of VideoReceiveStream.

* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were
registered in the ctor. (this was open to a use-after-free regression)

* Delay and callback pointers are now passed via the ctors to the
IncomingVideoStream classes.  The thread primitives in the IncomingVideoStream
classes are also constructed/destructed at the same time as the owning object,
which allowed me to remove one more lock.

* Removed code in the VideoStreamDecoder that could overwrite the
VideoReceiveStream render delay with a fixed value of 10ms on construction. 
This wasn't a problem with the previous implementation (it would be now though)
but seemed to me like the wrong place to be setting that value.

* Made the render delay value in VideoRenderFrames, const.

BUG=chromium:620232
TBR=mflodman

Committed: https://crrev.com/e03f8787377bbc03a4e00184bb14b7561b108cbb
Cr-Commit-Position: refs/heads/master@{#13175}
==========

[commit-bot: I haz the power, 2016-06-16 20:29:21]

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

[tommi, 2016-06-16 22:43:51]

A revert of this CL (patchset #4 id:290001) has been created in
https://codereview.webrtc.org/2071093002/ by tommi@webrtc.org.

The reason for reverting is: Reverting again.  The perf regression does not seem
to be related to dropping frames..