To allow end-to-end QuicDataChannel usage with a
PeerConnection, RTCConfiguration has been modified to
include a boolean for whether to do QUIC, since negotiation of
QUIC is not implemented. If one peer does QUIC, then it will be
assumed that the other peer must do QUIC or the connection
will fail.
PeerConnection has been modified to create data channels of type
QuicDataChannel when the peer wants to do QUIC.
WebRtcSession has ben modified to use a QuicDataTransport
instead of a DtlsTransportChannelWrapper/DataChannel
when QUIC should be used
QuicDataTransport implements the generic functions of
BaseChannel to manage the QuicTransportChannel.
Committed: https://crrev.com/34b54c36a533dadb6ceb70795119194e6f530ef5
Review-Url: https://codereview.webrtc.org/2166873002
Cr-Original-Commit-Position: refs/heads/master@{#13645}
Cr-Commit-Position: refs/heads/master@{#13657}
Reason for revert:
Reverting because it broke an RTP data channel test on the FYI bots.
Original issue's description:
> Modified PeerConnection and WebRtcSession for end-to-end QuicDataChannel usage.
>
> To allow end-to-end QuicDataChannel usage with a
> PeerConnection, RTCConfiguration has been modified to
> include a boolean for whether to do QUIC, since negotiation of
> QUIC is not implemented. If one peer does QUIC, then it will be
> assumed that the other peer must do QUIC or the connection
> will fail.
>
> PeerConnection has been modified to create data channels of type
> QuicDataChannel when the peer wants to do QUIC.
>
> WebRtcSession has ben modified to use a QuicDataTransport
> instead of a DtlsTransportChannelWrapper/DataChannel
> when QUIC should be used
>
> QuicDataTransport implements the generic functions of
> BaseChannel to manage the QuicTransportChannel.
>
> Committed: https://crrev.com/34b54c36a533dadb6ceb70795119194e6f530ef5
> Cr-Commit-Position: refs/heads/master@{#13645}
TBR=pthatcher@webrtc.org,zhihuang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.webrtc.org/2206793007
Cr-Commit-Position: refs/heads/master@{#13647}
To allow end-to-end QuicDataChannel usage with a
PeerConnection, RTCConfiguration has been modified to
include a boolean for whether to do QUIC, since negotiation of
QUIC is not implemented. If one peer does QUIC, then it will be
assumed that the other peer must do QUIC or the connection
will fail.
PeerConnection has been modified to create data channels of type
QuicDataChannel when the peer wants to do QUIC.
WebRtcSession has ben modified to use a QuicDataTransport
instead of a DtlsTransportChannelWrapper/DataChannel
when QUIC should be used
QuicDataTransport implements the generic functions of
BaseChannel to manage the QuicTransportChannel.
Review-Url: https://codereview.webrtc.org/2166873002
Cr-Commit-Position: refs/heads/master@{#13645}
This is part of rewriting the ConferenceMixer and OutputMixer.
Calls are instead routed through AudioReceiveStream::Start/Stop.
NOTRY=True
Review-Url: https://codereview.webrtc.org/2206223002
Cr-Commit-Position: refs/heads/master@{#13636}
Passing transport_frame_id() to VideoSink will allow to identify incoming video
frames, which will make it possible to correlate video frames on the
sender and on the receiver.
BUG=chromium:621691
R=mflodman@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2088953002 .
Cr-Commit-Position: refs/heads/master@{#13596}
This is a first CL wiring up AudioSendStream to BitrateAllocator. This
is still experimental and there is a test added for the audio only case,
combined audio video variable bitrate test cases will be added as a
follow up.
BUG=5079
Review-Url: https://codereview.webrtc.org/2165743003
Cr-Commit-Position: refs/heads/master@{#13527}
Reason for revert:
For some reason, payload_type_mapper.cc is not being picked up in Chrome builds, leading to undefined references. Reverting while investigating.
Original issue's description:
> WebRtcVoiceEngine: Use AudioDecoderFactory to generate recv codecs.
>
> Changed WebRtcVoiceEngine to present receive codecs from the formats
> provided by its decoder factory. Added supported formats to
> BuiltinAudioDecoderFactory. Added helper functions for creating some
> simple decoder factories for mocking.
>
> Created a PayloadTypeMapper for assigning payload types to formats. I
> think we'll eventually want to use this further up, or possibly in
> both the audio and video sides. It would be best if the engines didn't
> have to talk payload types at all, but it might be more difficult to
> get around when payload types depend on each-other, like the RTX
> codecs for video.
>
> This CL also includes some changes to rtc::Optional. I've put them in
> a separate CL that should (or should not) land first, making these
> changes void.
> See: https://codereview.webrtc.org/2072713002/
>
> BUG=webrtc:5805
>
> Committed: https://crrev.com/95eb1ba0db79d8fd134ae61b0a24648598684e8a
> Cr-Commit-Position: refs/heads/master@{#13459}
TBR=ivoc@webrtc.org,tina.legrand@webrtc.org,tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/2151453002
Cr-Commit-Position: refs/heads/master@{#13460}
Changed WebRtcVoiceEngine to present receive codecs from the formats
provided by its decoder factory. Added supported formats to
BuiltinAudioDecoderFactory. Added helper functions for creating some
simple decoder factories for mocking.
Created a PayloadTypeMapper for assigning payload types to formats. I
think we'll eventually want to use this further up, or possibly in
both the audio and video sides. It would be best if the engines didn't
have to talk payload types at all, but it might be more difficult to
get around when payload types depend on each-other, like the RTX
codecs for video.
This CL also includes some changes to rtc::Optional. I've put them in
a separate CL that should (or should not) land first, making these
changes void.
See: https://codereview.webrtc.org/2072713002/
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/2072753002
Cr-Commit-Position: refs/heads/master@{#13459}
webrtc doesnt include the header that the function is prototyped in.
This CL makes the convert_from.h include those headers to allow webrtc to
update to the head libyuv.
R=marpan@webrtc.org,pbos@webrtc.org
BUG=libyuv:620,webrtc:6094
TESTED=local build and try bots
Review-Url: https://codereview.webrtc.org/2139853002
Cr-Commit-Position: refs/heads/master@{#13436}
Reason for revert:
Reverting all CLs related to moving the eventlog, as they break Chromium tests.
Original issue's description:
> Move RtcEventLog object from inside VoiceEngine to Call.
>
> In addition to moving the logging object itself, this also moves the interface from PeerConnectionFactory to PeerConnection, which makes more sense for this functionality. An API parameter to set an upper limit to the size of the logfile is introduced.
> The old interface on PeerConnectionFactory is not removed in this CL, because it is called from Chrome, it will be removed after Chrome is updated to use the PeerConnection interface.
>
> BUG=webrtc:4741,webrtc:5603,chromium:609749
> R=solenberg@webrtc.org, stefan@webrtc.org, terelius@webrtc.org, tommi@webrtc.org
>
> Committed: https://crrev.com/1895526c6130e3d0e9b154f95079b8eda7567016
> Cr-Commit-Position: refs/heads/master@{#13321}
TBR=solenberg@webrtc.org,tommi@webrtc.org,stefan@webrtc.org,terelius@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:4741,webrtc:5603,chromium:609749
Review-Url: https://codereview.webrtc.org/2111813002
Cr-Commit-Position: refs/heads/master@{#13340}
functionality and exposes the functionality using the
MediaConstraints.
The exposing of the feature through the MediaConstraints
was done similarly to what was done for the intelligibility
enhancer in the CL
https://codereview.webrtc.org/1952123003
This CL is dependent on the CL https://codereview.webrtc.org/2090583002/ which contains
the level control functionality.
NOTRY=true
BUG=webrtc:5920
Review-Url: https://codereview.webrtc.org/2095563002
Cr-Commit-Position: refs/heads/master@{#13336}
In addition to moving the logging object itself, this also moves the interface from PeerConnectionFactory to PeerConnection, which makes more sense for this functionality. An API parameter to set an upper limit to the size of the logfile is introduced.
The old interface on PeerConnectionFactory is not removed in this CL, because it is called from Chrome, it will be removed after Chrome is updated to use the PeerConnection interface.
BUG=webrtc:4741,webrtc:5603,chromium:609749
R=solenberg@webrtc.org, stefan@webrtc.org, terelius@webrtc.org, tommi@webrtc.org
Review URL: https://codereview.webrtc.org/1748403002 .
Cr-Commit-Position: refs/heads/master@{#13321}
Relanding again after fixing issue with RTC_DCHECKs.
This CL eliminates the need for the extra layer of indirection provided by
mediastreamprovider.h. It will thus make it easier to implement new
functionality in RtpSender/RtpReceiver.
It also brings us one step closer to the end goal of combining "senders"
and "send streams". Currently the sender still needs to go through the
BaseChannel and MediaChannel, using an SSRC as a key.
R=pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/2046173002 .
Cr-Commit-Position: refs/heads/master@{#13305}
Reason for revert:
Broke video sending for iOS AppRTCDemo. To repro, run iOS AppRTCDemo in Release in loopback mode. The revision prior to this change worked.
Original issue's description:
> Reland of: Use VoiceChannel/VideoChannel directly from RtpSender/RtpReceiver.
>
> This eliminates the need for the extra layer of indirection provided by
> mediastreamprovider.h. It will thus make it easier to implement new
> functionality in RtpSender/RtpReceiver.
>
> It also brings us one step closer to the end goal of combining "senders"
> and "send streams". Currently the sender still needs to go through the
> BaseChannel and MediaChannel, using an SSRC as a key.
>
> R=pthatcher@webrtc.org
>
> Committed: 2d5491783aTBR=pthatcher@webrtc.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.webrtc.org/2092273003
Cr-Commit-Position: refs/heads/master@{#13289}
This eliminates the need for the extra layer of indirection provided by
mediastreamprovider.h. It will thus make it easier to implement new
functionality in RtpSender/RtpReceiver.
It also brings us one step closer to the end goal of combining "senders"
and "send streams". Currently the sender still needs to go through the
BaseChannel and MediaChannel, using an SSRC as a key.
R=pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/2046173002 .
Cr-Commit-Position: refs/heads/master@{#13287}
Reason for revert:
Broke peerconnection_unittest somehow, due to introduction of thread check. Will fix and reland.
Original issue's description:
> Use VoiceChannel/VideoChannel directly from RtpSender/RtpReceiver.
>
> This eliminates the need for the extra layer of indirection provided by
> mediastreamprovider.h. It will thus make it easier to implement new
> functionality in RtpSender/RtpReceiver.
>
> It also brings us one step closer to the end goal of combining "senders"
> and "send streams". Currently the sender still needs to go through the
> BaseChannel and MediaChannel, using an SSRC as a key.
>
> R=pthatcher@webrtc.org
>
> Committed: https://crrev.com/bc5831999d3354509d75357b659b4bb8e39f8a6c
> Cr-Commit-Position: refs/heads/master@{#13285}
TBR=pthatcher@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.webrtc.org/2099843003
Cr-Commit-Position: refs/heads/master@{#13286}
This eliminates the need for the extra layer of indirection provided by
mediastreamprovider.h. It will thus make it easier to implement new
functionality in RtpSender/RtpReceiver.
It also brings us one step closer to the end goal of combining "senders"
and "send streams". Currently the sender still needs to go through the
BaseChannel and MediaChannel, using an SSRC as a key.
R=pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/2046173002 .
Cr-Commit-Position: refs/heads/master@{#13285}
Reason for revert:
Breaking Chrome FYI bots.
Original issue's description:
> Cleanups in cricket::VideoFrame and cricket::WebRtcVideoFrame.
>
> Removed some protected virtual methods from VideoFrame that no longer
> need to exist. Some minor cleanups in the tests.
>
> BUG=webrtc:5682
>
> Committed: https://crrev.com/742d7b10b9720ec43de26e0faef52e5cb9c0daa8
> Cr-Commit-Position: refs/heads/master@{#13275}
TBR=pbos@webrtc.org,nisse@webrtc.org,deadbeef@webrtc.org,sergeyu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682
Review-Url: https://codereview.webrtc.org/2091983002
Cr-Commit-Position: refs/heads/master@{#13277}
Removed some protected virtual methods from VideoFrame that no longer
need to exist. Some minor cleanups in the tests.
BUG=webrtc:5682
Review-Url: https://codereview.webrtc.org/2075983003
Cr-Commit-Position: refs/heads/master@{#13275}
VideoDecoderParams contains the id of the receive video
stream. Motivation behind this change is to enable down
stream apps easier map raw non-decoded data to incoming
streams.
BUG=b/28636393
Review-Url: https://codereview.webrtc.org/2052233002
Cr-Commit-Position: refs/heads/master@{#13250}
Reason for revert:
Reverting the revert. This change is not related to the failure on the Windows FYI bots. The cause of the failure has been reverted in Chromium:
https://codereview.chromium.org/2081653004/
Original issue's description:
> Revert of Split IncomingVideoStream into two implementations, with smoothing and without. (patchset #5 id:340001 of https://codereview.webrtc.org/2078873002/ )
>
> Reason for revert:
> Breaks chromium.webrtc.fyi
>
> https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win7%20Tester/builds/4719
> https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win10%20Tester/builds/3120
>
> Original issue's description:
> > Reland of IncomingVideoStream refactoring.
> > This reland does not contain the non-smoothing part of the original implementation. Instead, when smoothing is turned off, frame callbacks run on the decoder thread, as they did before. This code path is used in Chrome. As far as Chrome goes, the difference now is that there won't be an instance of IncomingVideoStream in between the decoder and the callback (i.e. fewer locks). Other than that, no change for Chrome.
> >
> > Original issue's description (with non-smoothing references removed):
> >
> > Split IncomingVideoStream into two implementations, with smoothing and without.
> >
> > * Added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 6 locks.
> >
> > * 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.
> >
> > * 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 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
> > R=mflodman@webrtc.org, nisse@webrtc.org
> >
> > Committed: https://crrev.com/884c336c345d988974c2a69cea402b0fb8b07a63
> > Cr-Commit-Position: refs/heads/master@{#13219}
>
> TBR=nisse@webrtc.org,philipel@webrtc.org,mflodman@webrtc.org,tommi@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=chromium:620232
>
> Committed: https://crrev.com/a536bfe70de38fe877245317a7f0b00bcf69cbd0
> Cr-Commit-Position: refs/heads/master@{#13229}
TBR=nisse@webrtc.org,philipel@webrtc.org,mflodman@webrtc.org,sakal@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:620232
Review-Url: https://codereview.webrtc.org/2089613002
Cr-Commit-Position: refs/heads/master@{#13230}
Reason for revert:
Breaks chromium.webrtc.fyi
https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win7%20Tester/builds/4719https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win10%20Tester/builds/3120
Original issue's description:
> Reland of IncomingVideoStream refactoring.
> This reland does not contain the non-smoothing part of the original implementation. Instead, when smoothing is turned off, frame callbacks run on the decoder thread, as they did before. This code path is used in Chrome. As far as Chrome goes, the difference now is that there won't be an instance of IncomingVideoStream in between the decoder and the callback (i.e. fewer locks). Other than that, no change for Chrome.
>
> Original issue's description (with non-smoothing references removed):
>
> Split IncomingVideoStream into two implementations, with smoothing and without.
>
> * Added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 6 locks.
>
> * 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.
>
> * 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 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
> R=mflodman@webrtc.org, nisse@webrtc.org
>
> Committed: https://crrev.com/884c336c345d988974c2a69cea402b0fb8b07a63
> Cr-Commit-Position: refs/heads/master@{#13219}
TBR=nisse@webrtc.org,philipel@webrtc.org,mflodman@webrtc.org,tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:620232
Review-Url: https://codereview.webrtc.org/2084873002
Cr-Commit-Position: refs/heads/master@{#13229}
This reland does not contain the non-smoothing part of the original implementation. Instead, when smoothing is turned off, frame callbacks run on the decoder thread, as they did before. This code path is used in Chrome. As far as Chrome goes, the difference now is that there won't be an instance of IncomingVideoStream in between the decoder and the callback (i.e. fewer locks). Other than that, no change for Chrome.
Original issue's description (with non-smoothing references removed):
Split IncomingVideoStream into two implementations, with smoothing and without.
* Added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 6 locks.
* 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.
* 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 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
R=mflodman@webrtc.org, nisse@webrtc.org
Review URL: https://codereview.webrtc.org/2078873002 .
Cr-Commit-Position: refs/heads/master@{#13219}
Don't use VideoFrameBuffer::MutableDataY and friends, instead, use
I420Buffer::SetToBlack.
Also introduce static method I420Buffer::Create, to create an object and
return a scoped_refptr.
TBR=marpan@webrtc.org # Trivial change to video_denoiser.cc
BUG=webrtc:5921
Review-Url: https://codereview.webrtc.org/2078943002
Cr-Commit-Position: refs/heads/master@{#13212}
Removes the need to use VoEVolume::SetChannelOutputVolumeScaling().
BUG=webrtc:4690
Review-Url: https://codereview.webrtc.org/2062193002
Cr-Commit-Position: refs/heads/master@{#13194}
Reason for revert:
Reverting again. The perf regression does not seem to be related to dropping frames.
Original issue's description:
> 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}
TBR=mflodman@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:620232
Review-Url: https://codereview.webrtc.org/2071093002
Cr-Commit-Position: refs/heads/master@{#13176}
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
Review-Url: https://codereview.webrtc.org/2071473002
Cr-Commit-Position: refs/heads/master@{#13175}
