[chrome.displaySource] Add a base class for audio and video encoders.

[chrome.displaySource] Add a base class for audio and video encoders.

This CL adds a new base class to be used as a common base class for
audio and video encoders and moves some of the functionality of the
video encoder base class to that new common base class.

This CL also renames the encoded unit class from WiFiDisplayEncodedFrame
to WiFiDisplayEncodedUnit so that the name makes sense for audio units,
too, adds constructors to that class and makes it to be a move-only
class in order to avoid unnecessary copies.

BUG=242107
Committed: https://crrev.com/751cb149434ddd7874106dbde6d3e8bc725bad62
Cr-Commit-Position: refs/heads/master@{#389049}

[e_hakkinen, 2016-04-20 18:33:08]

eero.hakkinen@intel.com changed reviewers:
+ alexander.shalamov@intel.com, asargent@chromium.org,
mikhail.pozdnyakov@intel.com

[e_hakkinen, 2016-04-20 18:33:08]

PTAL. This is a prerequisite for further audio and video encoder CLs.

[e_hakkinen, 2016-04-20 18:53:16]

On 2016/04/20 18:33:08, e_hakkinen wrote:
> This is a prerequisite for further audio CLs

See https://codereview.chromium.org/1899943005/

> and video encoder CLs.

See https://codereview.chromium.org/1905673002/

[Mikhail, 2016-04-20 18:58:31]

lgtm with nits

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h
(right):

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h:21:
public:
nit: 'public' is not needed in struct

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h:29:
WiFiDisplayEncodedUnit(const WiFiDisplayEncodedUnit&) = delete;
DISALLOW_COPY_AND_ASSIGN would be better

[e_hakkinen, 2016-04-20 19:22:53]

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h
(right):

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h:29:
WiFiDisplayEncodedUnit(const WiFiDisplayEncodedUnit&) = delete;
On 2016/04/20 18:58:31, Mikhail wrote:
> DISALLOW_COPY_AND_ASSIGN would be better

IMHO, the problem with DISALLOW_COPY_AND_ASSIGN is that it can only be used in
the private: declarion of a class, and I would like to avoid access specifiers
altogether in a struct. WDYT?

[Mikhail, 2016-04-20 19:26:21]

On 2016/04/20 19:22:53, e_hakkinen wrote:
>
https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
> File
>
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h
> (right):
> 
>
https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
>
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h:29:
> WiFiDisplayEncodedUnit(const WiFiDisplayEncodedUnit&) = delete;
> On 2016/04/20 18:58:31, Mikhail wrote:
> > DISALLOW_COPY_AND_ASSIGN would be better
> 
> IMHO, the problem with DISALLOW_COPY_AND_ASSIGN is that it can only be used in
> the private: declarion of a class, and I would like to avoid access specifiers
> altogether in a struct. WDYT?

DISALLOW_COPY_AND_ASSIGN will end up in declaration deleted ctor and assignment
operator (same as you're doing here) I think it can used in struct even without
an access specifier.

[e_hakkinen, 2016-04-21 10:24:36]

On 2016/04/20 19:26:21, Mikhail wrote:
> DISALLOW_COPY_AND_ASSIGN will end up in declaration deleted ctor and
assignment
> operator (same as you're doing here) I think it can used in struct even
without
> an access specifier.

Surprisingly it does not. There is an inconsistency between
DISALLOW_COPY_AND_ASSIGN and DISALLOW_COPY/DISALLOW_ASSIGN. DISALLOW_COPY and
DISALLOW_ASSIGN delete copy ctor and assignment operators (using C++11 syntax)
whereas DISALLOW_COPY_AND_ASSIGN does not delete anything but declares copy ctor
and assignment operator. The idea is that DISALLOW_COPY_AND_ASSIGN is used in
private: (that is checked by git cl lint) and that those declared copy ctor and
assignment operator are not defined anythere so that declared copy ctor and
assignment operator cannot be used. So DISALLOW_COPY_AND_ASSIGN predates C++11
syntax.

[e_hakkinen, 2016-04-21 10:25:30]

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h
(right):

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h:21:
public:
On 2016/04/20 18:58:31, Mikhail wrote:
> nit: 'public' is not needed in struct

Done.

https://codereview.chromium.org/1903773003/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_media_encoder.h:29:
WiFiDisplayEncodedUnit(const WiFiDisplayEncodedUnit&) = delete;
I'll use DISALLOW_ASSIGN and DISALLOW_COPY.

[asargent_no_longer_on_chrome, 2016-04-21 23:41:31]

lgtm

[e_hakkinen, 2016-04-22 06:25:07]

The CQ bit was checked by eero.hakkinen@intel.com

[e_hakkinen, 2016-04-22 06:25:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
mikhail.pozdnyakov@intel.com
Link to the patchset: https://codereview.chromium.org/1903773003/#ps20001
(title: "Use DISALLOW_{ASSIGN,COPY}")

[commit-bot: I haz the power, 2016-04-22 06:25:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1903773003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1903773003/20001

[commit-bot: I haz the power, 2016-04-22 07:10:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-22 19:45:13]

Description was changed from

==========
[chrome.displaySource] Add a base class for audio and video encoders.

This CL adds a new base class to be used as a common base class for
audio and video encoders and moves some of the functionality of the
video encoder base class to that new common base class.

This CL also renames the encoded unit class from WiFiDisplayEncodedFrame
to WiFiDisplayEncodedUnit so that the name makes sense for audio units,
too, adds constructors to that class and makes it to be a move-only
class in order to avoid unnecessary copies.

BUG=242107
==========

to

==========
[chrome.displaySource] Add a base class for audio and video encoders.

This CL adds a new base class to be used as a common base class for
audio and video encoders and moves some of the functionality of the
video encoder base class to that new common base class.

This CL also renames the encoded unit class from WiFiDisplayEncodedFrame
to WiFiDisplayEncodedUnit so that the name makes sense for audio units,
too, adds constructors to that class and makes it to be a move-only
class in order to avoid unnecessary copies.

BUG=242107

Committed: https://crrev.com/751cb149434ddd7874106dbde6d3e8bc725bad62
Cr-Commit-Position: refs/heads/master@{#389049}
==========

[commit-bot: I haz the power, 2016-04-22 19:45:14]

Patchset 2 (id:??) landed as
https://crrev.com/751cb149434ddd7874106dbde6d3e8bc725bad62
Cr-Commit-Position: refs/heads/master@{#389049}