Dry run: Try jobs failed on following builders: ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/builds/500) ios64_sim_ios10_dbg on ...
3 years, 11 months ago
(2017-01-19 09:54:44 UTC)
#4
asapersson@ is ooo +magjed, can you take a look as you're an owner of webrtc/media? ...
3 years, 11 months ago
(2017-01-23 09:41:56 UTC)
#12
asapersson@ is ooo
+magjed, can you take a look as you're an owner of webrtc/media?
I'd like to land this asap.
magjed_webrtc
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrtcvideoengine2.cc File webrtc/media/engine/webrtcvideoengine2.cc (right): https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrtcvideoengine2.cc#newcode1679 webrtc/media/engine/webrtcvideoengine2.cc:1679: WebRtcSimulcastEncoderFactory adapter_factory( Do we really need to have a ...
3 years, 11 months ago
(2017-01-23 10:36:24 UTC)
#13
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrtcvideoengine2.cc File webrtc/media/engine/webrtcvideoengine2.cc (right): https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrtcvideoengine2.cc#newcode1679 webrtc/media/engine/webrtcvideoengine2.cc:1679: WebRtcSimulcastEncoderFactory adapter_factory( On 2017/01/23 10:36:24, magjed_webrtc wrote: > Do ...
3 years, 11 months ago
(2017-01-23 12:09:32 UTC)
#14
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1679: WebRtcSimulcastEncoderFactory
adapter_factory(
On 2017/01/23 10:36:24, magjed_webrtc wrote:
> Do we really need to have a special case for screenshare simulcast here? Can't
> you add it to where simulcast is created for non-screenshare?
What do you mean by "where simulcast is created"?
In the case of libvpx + simulcast, the codec has built in simulcast support, but
it does not work if the layers have the same resolution or different framerate,
so we must use completely independent codec instances for each simulcast stream.
That's the purpose of encoder wrappers created by WebRtcSimulcastEncoderFactory.
I'd rather not duplicate that logic somewhere else?
sprang_webrtc
ping?
3 years, 11 months ago
(2017-01-24 09:55:06 UTC)
#15
3 years, 11 months ago
(2017-01-24 09:58:52 UTC)
#16
lgtm
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1679: WebRtcSimulcastEncoderFactory
adapter_factory(
On 2017/01/23 12:09:32, språng wrote:
> On 2017/01/23 10:36:24, magjed_webrtc wrote:
> > Do we really need to have a special case for screenshare simulcast here?
Can't
> > you add it to where simulcast is created for non-screenshare?
>
> What do you mean by "where simulcast is created"?
>
> In the case of libvpx + simulcast, the codec has built in simulcast support,
but
> it does not work if the layers have the same resolution or different
framerate,
> so we must use completely independent codec instances for each simulcast
stream.
> That's the purpose of encoder wrappers created by
WebRtcSimulcastEncoderFactory.
> I'd rather not duplicate that logic somewhere else?
Ok, I didn't know how simulcast was created in other cases. For the external
factory, it looks like we always wrap it in a WebRtcSimulcastEncoderFactory. The
reason I'm asking is because we have a long-term idea to remove the internal
codec factory and only have one (external) factory that the client sets in the
CreatePeerConnectionFactory call. The purpose with that would be to make the
code more modular so that we don't have references from internal WebRTC to
specific codecs, and the client can choose to include or not include whatever
software codecs it wants. This change makes the code for the internal factory
and the external factory slightly more different, but I guess that's fine.
3 years, 10 months ago
(2017-01-24 13:06:10 UTC)
#17
On 2017/01/24 09:58:52, magjed_webrtc wrote:
> lgtm
>
>
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrt...
> File webrtc/media/engine/webrtcvideoengine2.cc (right):
>
>
https://codereview.webrtc.org/2641133002/diff/60001/webrtc/media/engine/webrt...
> webrtc/media/engine/webrtcvideoengine2.cc:1679: WebRtcSimulcastEncoderFactory
> adapter_factory(
> On 2017/01/23 12:09:32, språng wrote:
> > On 2017/01/23 10:36:24, magjed_webrtc wrote:
> > > Do we really need to have a special case for screenshare simulcast here?
> Can't
> > > you add it to where simulcast is created for non-screenshare?
> >
> > What do you mean by "where simulcast is created"?
> >
> > In the case of libvpx + simulcast, the codec has built in simulcast support,
> but
> > it does not work if the layers have the same resolution or different
> framerate,
> > so we must use completely independent codec instances for each simulcast
> stream.
> > That's the purpose of encoder wrappers created by
> WebRtcSimulcastEncoderFactory.
> > I'd rather not duplicate that logic somewhere else?
>
> Ok, I didn't know how simulcast was created in other cases. For the external
> factory, it looks like we always wrap it in a WebRtcSimulcastEncoderFactory.
The
> reason I'm asking is because we have a long-term idea to remove the internal
> codec factory and only have one (external) factory that the client sets in the
> CreatePeerConnectionFactory call. The purpose with that would be to make the
> code more modular so that we don't have references from internal WebRTC to
> specific codecs, and the client can choose to include or not include whatever
> software codecs it wants. This change makes the code for the internal factory
> and the external factory slightly more different, but I guess that's fine.
Right, thanks. Sounds like we might need to make the interface for the encoder
factories a bit more ugly then :/
sprang_webrtc
The CQ bit was checked by sprang@webrtc.org
3 years, 10 months ago
(2017-01-24 13:06:15 UTC)
#18
Try jobs failed on following builders: linux_asan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/21349)
3 years, 10 months ago
(2017-01-24 13:08:49 UTC)
#21
Try jobs failed on following builders: linux_memcheck on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/4460)
3 years, 10 months ago
(2017-01-24 14:17:51 UTC)
#26
3 years, 10 months ago
(2017-01-24 19:18:48 UTC)
#30
Dry run: This issue passed the CQ dry run.
sprang_webrtc
Fixed a memory leak in the unit test, do you wanna have a look or ...
3 years, 10 months ago
(2017-01-25 15:08:13 UTC)
#31
Fixed a memory leak in the unit test, do you wanna have a look or should it hit
the trigger?
magjed_webrtc
On 2017/01/25 15:08:13, språng wrote: > Fixed a memory leak in the unit test, do ...
3 years, 10 months ago
(2017-01-25 18:59:59 UTC)
#32
On 2017/01/25 15:08:13, språng wrote:
> Fixed a memory leak in the unit test, do you wanna have a look or should it
hit
> the trigger?
Go ahead and commit.
sprang_webrtc
The CQ bit was checked by sprang@webrtc.org
3 years, 10 months ago
(2017-01-26 07:44:53 UTC)
#33
Issue 2641133002: Reland of Add experimental simulcast screen content mode
(Closed)
Created 3 years, 11 months ago by sprang_webrtc
Modified 3 years, 10 months ago
Reviewers: magjed_webrtc
Base URL:
Comments: 3