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

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.
 * https://codereview.chromium.org/1905243003/
   Integrate audio encoders into media pipeline.

BUG=242107
Committed: https://crrev.com/88ad383927af2ec678a5e9cf6fe2e0ebcd2d0184
Cr-Commit-Position: refs/heads/master@{#390036}

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

Description was changed from

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

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

to

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.

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

[e_hakkinen, 2016-04-20 18:51:59]

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

[e_hakkinen, 2016-04-22 09:13:59]

Description was changed from

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.

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

to

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.
 * https://codereview.chromium.org/1905243003/
   Integrate audio encoders into media pipeline.

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

[e_hakkinen, 2016-04-22 09:19:03]

PTAL.

[Mikhail, 2016-04-22 09:38:22]

https://codereview.chromium.org/1899943005/diff/1/extensions/renderer/api/dis...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.cc
(right):

https://codereview.chromium.org/1899943005/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.cc:31:
return ~static_cast<size_t>(0u);
let's add 'static const size_t kInvalidCodecModeValue =
~static_cast<size_t>(0u);' to the class declaration

[e_hakkinen, 2016-04-22 10:31:51]

https://codereview.chromium.org/1899943005/diff/1/extensions/renderer/api/dis...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.cc
(right):

https://codereview.chromium.org/1899943005/diff/1/extensions/renderer/api/dis...
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.cc:31:
return ~static_cast<size_t>(0u);
On 2016/04/22 09:38:22, Mikhail wrote:
> let's add 'static const size_t kInvalidCodecModeValue =
> ~static_cast<size_t>(0u);' to the class declaration

Done.

[Mikhail, 2016-04-22 10:36:33]

lgtm with a minor comment

https://codereview.chromium.org/1899943005/diff/20001/extensions/renderer/api...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h
(right):

https://codereview.chromium.org/1899943005/diff/20001/extensions/renderer/api...
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h:23:
static const size_t kInvalidCodecModeValue = ~static_cast<size_t>(0u);
think it should be protected (same as GetAudioCodecMode())

[e_hakkinen, 2016-04-22 10:39:24]

https://codereview.chromium.org/1899943005/diff/20001/extensions/renderer/api...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h
(right):

https://codereview.chromium.org/1899943005/diff/20001/extensions/renderer/api...
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h:23:
static const size_t kInvalidCodecModeValue = ~static_cast<size_t>(0u);
On 2016/04/22 10:36:33, Mikhail wrote:
> think it should be protected (same as GetAudioCodecMode())

True.

[shalamov, 2016-04-22 13:27:11]

lgtm

[asargent_no_longer_on_chrome, 2016-04-26 19:06:05]

lgtm

https://codereview.chromium.org/1899943005/diff/40001/extensions/renderer/api...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h
(right):

https://codereview.chromium.org/1899943005/diff/40001/extensions/renderer/api...
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h:16:
// Threading: the client code should belong to a single thread.
optional: if you think clients of this class might get this wrong, you can
enforce this using base::ThreadChecker and DCHECKS using ThreadChecker's
CalledOnValidThread method

[e_hakkinen, 2016-04-27 09:08:23]

https://codereview.chromium.org/1899943005/diff/40001/extensions/renderer/api...
File
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h
(right):

https://codereview.chromium.org/1899943005/diff/40001/extensions/renderer/api...
extensions/renderer/api/display_source/wifi_display/wifi_display_audio_encoder.h:16:
// Threading: the client code should belong to a single thread.
On 2016/04/26 19:06:05, Antony Sargent wrote:
> optional: if you think clients of this class might get this wrong, you can
> enforce this using base::ThreadChecker and DCHECKS using ThreadChecker's
> CalledOnValidThread method

I think it is more likely for clients to get this wrong for video encoders than
for audio encoders as audio encoders have smaller client interface compared to
video encoders. As such, I am inclined to add a protected thread checker to the
media encoder base class and reuse that in all encoders. I will do that in a
separate CL.

[e_hakkinen, 2016-04-27 09:08:30]

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

[e_hakkinen, 2016-04-27 09:08:30]

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/1899943005/#ps40001
(title: "Proper access for error constant")

[commit-bot: I haz the power, 2016-04-27 09:08:39]

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

[commit-bot: I haz the power, 2016-04-27 09:49:47]

Description was changed from

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.
 * https://codereview.chromium.org/1905243003/
   Integrate audio encoders into media pipeline.

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

to

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.
 * https://codereview.chromium.org/1905243003/
   Integrate audio encoders into media pipeline.

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

[commit-bot: I haz the power, 2016-04-27 09:49:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-27 09:51:16]

Description was changed from

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.
 * https://codereview.chromium.org/1905243003/
   Integrate audio encoders into media pipeline.

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

to

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

This CL adds a new base class to be used as a common base class for all
audio encoders.

This part of a Wi-Fi Display audo encoder patch series:
 * https://codereview.chromium.org/1899943005/  <-- This CL
   Add a base class for audio encoders.
 * https://codereview.chromium.org/1903823002/
   Implement LPCM audio encoder.
 * https://codereview.chromium.org/1905243003/
   Integrate audio encoders into media pipeline.

BUG=242107

Committed: https://crrev.com/88ad383927af2ec678a5e9cf6fe2e0ebcd2d0184
Cr-Commit-Position: refs/heads/master@{#390036}
==========

[commit-bot: I haz the power, 2016-04-27 09:51:17]

Patchset 3 (id:??) landed as
https://crrev.com/88ad383927af2ec678a5e9cf6fe2e0ebcd2d0184
Cr-Commit-Position: refs/heads/master@{#390036}