Remove webrtc::VideoEncoderFactory

Remove webrtc::VideoEncoderFactory

Replace the use of webrtc::VideoEncoderFactory with
cricket::WebRtcVideoEncoderFactory and remove the adapter classes
between these two factory types.

Some code changes were necessary in order to accomplish this:
 * Move SimulcastEncoderAdapter from
   webrtc/modules/video_coding/codecs/vp8 to webrtc/media/engine (that's
   where it's used).
 * Rename simulcast_unittest.h to simulcast_test_utility.h and make it
   into it's own target, because it's used from both
   simulcast_unittest.cc and simulcast_encoder_adapter_unittest.cc.
 * Remove ownership of the encoder factory from SimulcastEncoderAdapter,
   and make the necessary changes in surrounding code.

The goal with this CL is to clean up the code, and also to free up
the name webrtc::VideoEncoderFactory for future use.

BUG=webrtc:7925
Review-Url: https://codereview.webrtc.org/2964953002
Cr-Commit-Position: refs/heads/master@{#18945}
Committed: https://chromium.googlesource.com/external/webrtc/+/6cc25614a9eaa068951f549d400645b471127b29

[magjed_webrtc, 2017-07-04 13:45:58]

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

[commit-bot: I haz the power, 2017-07-04 13:46:02]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-04 13:48:12]

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

[commit-bot: I haz the power, 2017-07-04 13:48:13]

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/buil...)
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/21709)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/27025)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/26030)

[magjed_webrtc, 2017-07-05 08:17:11]

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

[commit-bot: I haz the power, 2017-07-05 08:17:13]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-05 08:19:23]

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

[commit-bot: I haz the power, 2017-07-05 08:19:24]

Dry run: Try jobs failed on following builders:
  mac_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/26027)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/22869)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/27128)

[magjed_webrtc, 2017-07-05 08:58:37]

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

[commit-bot: I haz the power, 2017-07-05 08:58:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-05 09:02:53]

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

[commit-bot: I haz the power, 2017-07-05 09:02:54]

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

[magjed_webrtc, 2017-07-05 09:08:39]

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

[commit-bot: I haz the power, 2017-07-05 09:08:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-05 09:25:11]

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

[commit-bot: I haz the power, 2017-07-05 09:25:12]

Dry run: 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/26102)

[magjed_webrtc, 2017-07-05 09:59:27]

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

[commit-bot: I haz the power, 2017-07-05 09:59:31]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-05 10:08:40]

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

[commit-bot: I haz the power, 2017-07-05 10:08:41]

Dry run: 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/26108)
  linux_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/27527)

[magjed_webrtc, 2017-07-05 12:04:07]

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

[commit-bot: I haz the power, 2017-07-05 12:04:13]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-05 12:25:23]

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

[commit-bot: I haz the power, 2017-07-05 12:25:24]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-07-05 12:30:14]

Patchset #5 (id:80001) has been deleted

[magjed_webrtc, 2017-07-05 12:30:21]

Patchset #4 (id:60001) has been deleted

[magjed_webrtc, 2017-07-05 12:30:30]

Patchset #3 (id:40001) has been deleted

[magjed_webrtc, 2017-07-05 12:30:36]

Patchset #2 (id:20001) has been deleted

[magjed_webrtc, 2017-07-05 12:30:45]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2017-07-05 12:40:41]

Description was changed from

==========
Remove webrtc::VideoEncoderFactory

Replace it's use with WebRtcVideoEncoderFactory instead.

BUG=webrtc:7925
==========

to

==========
Remove webrtc::VideoEncoderFactory

Replace the use of webrtc::VideoEncoderFactory with
cricket::WebRtcVideoEncoderFactory and remove the adapter classes
between these two factory types.

Some code changes were necessary in order to accomplish this:
 * Move SimulcastEncoderAdapter from
   webrtc/modules/video_coding/codecs/vp8 to webrtc/media/engine (that's
   where it's used).
 * Rename simulcast_unittest.h to simulcast_test_utility.h and make it
   into it's own target, because it's used from both
   simulcast_unittest.cc and simulcast_encoder_adapter_unittest.cc.
 * Remove ownership of the encoder factory from SimulcastEncoderAdapter,
   and make the necessary changes in all usages.

BUG=webrtc:7925
==========

[magjed_webrtc, 2017-07-05 12:40:41]

magjed@webrtc.org changed reviewers:
+ stefan@webrtc.org

[magjed_webrtc, 2017-07-05 12:42:20]

Description was changed from

==========
Remove webrtc::VideoEncoderFactory

Replace the use of webrtc::VideoEncoderFactory with
cricket::WebRtcVideoEncoderFactory and remove the adapter classes
between these two factory types.

Some code changes were necessary in order to accomplish this:
 * Move SimulcastEncoderAdapter from
   webrtc/modules/video_coding/codecs/vp8 to webrtc/media/engine (that's
   where it's used).
 * Rename simulcast_unittest.h to simulcast_test_utility.h and make it
   into it's own target, because it's used from both
   simulcast_unittest.cc and simulcast_encoder_adapter_unittest.cc.
 * Remove ownership of the encoder factory from SimulcastEncoderAdapter,
   and make the necessary changes in all usages.

BUG=webrtc:7925
==========

to

==========
Remove webrtc::VideoEncoderFactory

Replace the use of webrtc::VideoEncoderFactory with
cricket::WebRtcVideoEncoderFactory and remove the adapter classes
between these two factory types.

Some code changes were necessary in order to accomplish this:
 * Move SimulcastEncoderAdapter from
   webrtc/modules/video_coding/codecs/vp8 to webrtc/media/engine (that's
   where it's used).
 * Rename simulcast_unittest.h to simulcast_test_utility.h and make it
   into it's own target, because it's used from both
   simulcast_unittest.cc and simulcast_encoder_adapter_unittest.cc.
 * Remove ownership of the encoder factory from SimulcastEncoderAdapter,
   and make the necessary changes in surrounding code.

The goal with this CL is to clean up the code, and also to free up
the name webrtc::VideoEncoderFactory for future use.

BUG=webrtc:7925
==========

[magjed_webrtc, 2017-07-05 12:42:42]

Stefan - please take a look.

[stefan-webrtc, 2017-07-07 09:55:22]

I don't have many comments, overall it looks good.

What I kind of dislike a bit is that we seem to mix cricket and webrtc namespace
a lot here. Is there some class that should be moved to the other namespace to
avoid this?

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine.cc (right):

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine.cc:38: #include
"webrtc/media/engine/simulcast_encoder_adapter.h"
Sort

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/vp8/simulcast_test_utility.h (right):

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp8/simulcast_test_utility.h:256: encoder_ =
nullptr;
encoder_.reset() is what I most commonly see. Maybe stick with that pattern?

[magjed_webrtc, 2017-07-07 12:07:51]

On 2017/07/07 09:55:22, stefan-webrtc wrote:
> I don't have many comments, overall it looks good.
> 
> What I kind of dislike a bit is that we seem to mix cricket and webrtc
namespace
> a lot here. Is there some class that should be moved to the other namespace to
> avoid this?

Most of the mixing is from the fact that we use
cricket::WebRtcVideoEncoderFactory to create webrtc::VideoEncoders, which
doesn't make sense. As part of webrtc:7925, I want to deprecate
cricket::WebRtcVideoEncoderFactory and use a new encoder factory class in the
webrtc namespace with a better interface. That should improve the namespace
mixing as well. I want to name the new class webrtc::VideoEncoderFactory, that's
why I'm doing this CL :).

[magjed_webrtc, 2017-07-07 12:08:00]

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine.cc (right):

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine.cc:38: #include
"webrtc/media/engine/simulcast_encoder_adapter.h"
On 2017/07/07 09:55:21, stefan-webrtc wrote:
> Sort

Done.

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/vp8/simulcast_test_utility.h (right):

https://codereview.webrtc.org/2964953002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/vp8/simulcast_test_utility.h:256: encoder_ =
nullptr;
On 2017/07/07 09:55:21, stefan-webrtc wrote:
> encoder_.reset() is what I most commonly see. Maybe stick with that pattern?

Done.

[stefan-webrtc, 2017-07-07 12:19:10]

On 2017/07/07 12:07:51, magjed_webrtc wrote:
> On 2017/07/07 09:55:22, stefan-webrtc wrote:
> > I don't have many comments, overall it looks good.
> > 
> > What I kind of dislike a bit is that we seem to mix cricket and webrtc
> namespace
> > a lot here. Is there some class that should be moved to the other namespace
to
> > avoid this?
> 
> Most of the mixing is from the fact that we use
> cricket::WebRtcVideoEncoderFactory to create webrtc::VideoEncoders, which
> doesn't make sense. As part of webrtc:7925, I want to deprecate
> cricket::WebRtcVideoEncoderFactory and use a new encoder factory class in the
> webrtc namespace with a better interface. That should improve the namespace
> mixing as well. I want to name the new class webrtc::VideoEncoderFactory,
that's
> why I'm doing this CL :).

Great! Lgtm

[magjed_webrtc, 2017-07-07 15:19:17]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2017-07-07 15:19:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-07 15:21:03]

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

[commit-bot: I haz the power, 2017-07-07 15:21:04]

Try jobs failed on following builders:
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/21810)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/27122)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[magjed_webrtc, 2017-07-10 09:04:19]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2017-07-10 09:04:20]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2964953002/#ps140001 (title:
"Rebase")

[commit-bot: I haz the power, 2017-07-10 09:04:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-10 09:06:15]

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

[commit-bot: I haz the power, 2017-07-10 09:06:16]

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/buil...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/21844)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/21630)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[magjed_webrtc, 2017-07-10 09:15:35]

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

[commit-bot: I haz the power, 2017-07-10 09:15:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-10 09:52:38]

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

[commit-bot: I haz the power, 2017-07-10 09:52:40]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2017-07-10 10:23:45]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2017-07-10 10:23:45]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2964953002/#ps160001 (title:
"Add dep to base:sequenced_task_checker")

[commit-bot: I haz the power, 2017-07-10 10:23:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-10 10:26:37]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1499682225246430,
"parent_rev": "c453b08f1e2ff8e731a2004780be40ec81f23304", "commit_rev":
"6cc25614a9eaa068951f549d400645b471127b29"}

[commit-bot: I haz the power, 2017-07-10 10:26:44]

Description was changed from

==========
Remove webrtc::VideoEncoderFactory

Replace the use of webrtc::VideoEncoderFactory with
cricket::WebRtcVideoEncoderFactory and remove the adapter classes
between these two factory types.

Some code changes were necessary in order to accomplish this:
 * Move SimulcastEncoderAdapter from
   webrtc/modules/video_coding/codecs/vp8 to webrtc/media/engine (that's
   where it's used).
 * Rename simulcast_unittest.h to simulcast_test_utility.h and make it
   into it's own target, because it's used from both
   simulcast_unittest.cc and simulcast_encoder_adapter_unittest.cc.
 * Remove ownership of the encoder factory from SimulcastEncoderAdapter,
   and make the necessary changes in surrounding code.

The goal with this CL is to clean up the code, and also to free up
the name webrtc::VideoEncoderFactory for future use.

BUG=webrtc:7925
==========

to

==========
Remove webrtc::VideoEncoderFactory

Replace the use of webrtc::VideoEncoderFactory with
cricket::WebRtcVideoEncoderFactory and remove the adapter classes
between these two factory types.

Some code changes were necessary in order to accomplish this:
 * Move SimulcastEncoderAdapter from
   webrtc/modules/video_coding/codecs/vp8 to webrtc/media/engine (that's
   where it's used).
 * Rename simulcast_unittest.h to simulcast_test_utility.h and make it
   into it's own target, because it's used from both
   simulcast_unittest.cc and simulcast_encoder_adapter_unittest.cc.
 * Remove ownership of the encoder factory from SimulcastEncoderAdapter,
   and make the necessary changes in surrounding code.

The goal with this CL is to clean up the code, and also to free up
the name webrtc::VideoEncoderFactory for future use.

BUG=webrtc:7925

Review-Url: https://codereview.webrtc.org/2964953002
Cr-Commit-Position: refs/heads/master@{#18945}
Committed:
https://chromium.googlesource.com/external/webrtc/+/6cc25614a9eaa068951f549d4...
==========

[commit-bot: I haz the power, 2017-07-10 10:26:45]

Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/external/webrtc/+/6cc25614a9eaa068951f549d4...