Mix to an intermediate buffer.

Mix to an intermediate buffer.

Mix audio into an intermediate buffer of 16 bit signed integers stored
in 32 bit values.  Do not clip and convert to the final output format
until all of the input tracks have been accumulated into the
intermediate mix buffer.

R=jeffbrown@google.com, dalesat@chromium.org
BUG=

Committed: https://chromium.googlesource.com/external/mojo/+/8d428b829a8b5efbe7f4bf33a329a3ae7226958c

[jeffbrown, 2015-12-01 22:13:31]

lgtm

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
File services/media/audio/platform/generic/output_formatter.cc (right):

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
services/media/audio/platform/generic/output_formatter.cc:81: register int32_t
val = source[i];
Does the register keyword even do anything these days?

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
services/media/audio/platform/generic/output_formatter.cc:97: //
Constructor/destructor for the bommon OutputFormatter base class.
common

[dalesat, 2016-01-24 19:07:39]

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
File services/media/audio/platform/generic/output_formatter.cc (right):

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
services/media/audio/platform/generic/output_formatter.cc:29: 
Can we just define a template function and specialize? Why is the Enable type
parameter needed?

template <typename DType> inline DType DstConvert(int32_t sample);

inline uint8_t DstConvert(in32_t sample) {
  return static_cast<uint8_t>((sample >> 8) + 0x80);
}

[johngro, 2016-01-25 18:12:32]

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
File services/media/audio/platform/generic/output_formatter.cc (right):

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
services/media/audio/platform/generic/output_formatter.cc:29: 
On 2016/01/24 19:07:39, dalesat wrote:
> Can we just define a template function and specialize? Why is the Enable type
> parameter needed?
> 
> template <typename DType> inline DType DstConvert(int32_t sample);
> 
> inline uint8_t DstConvert(in32_t sample) {
>   return static_cast<uint8_t>((sample >> 8) + 0x80);
> }
>  

Its been a while, but this can fail in some cases when using type traits to do
the specialization.  IIRC, its something about not referencing all of the
template parameters in the specialization list directly which then causes the
compiler to complain with

"""
warning: class template partial specialization contains a template parameter
that cannot be deduced; this partial specialization will never be used
"""

I'd have to get back into the problem to really understand it and provide a
proper explanation; but the "Add one last parameter to the template" pattern (in
this case, the Enable parameter) is a common way of getting around the trouble. 
There are others, but whether or not they work depend on the specifics of the
situation.

I found this pattern to be reliable, and generally more readable than the
alternatives (not that C++ partial specialization syntax lends itself to
read-ability), so I apply it pretty much everywhere here.  It may not be needed
in this specific case, I can remove it if it offends, or at least page the
problem back into my brain and provide a more precise answer as to the details
about partial template parameter deduction which would explain why this needed a
little bit better.

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
services/media/audio/platform/generic/output_formatter.cc:81: register int32_t
val = source[i];
On 2015/12/01 22:13:30, jeffbrown wrote:
> Does the register keyword even do anything these days?

probably not, but it does not hurt either.  It has always been just a suggestion
of intent to the compiler, I can remove it if it bugs you or is confusing.

https://codereview.chromium.org/1471813002/diff/20001/services/media/audio/pl...
services/media/audio/platform/generic/output_formatter.cc:97: //
Constructor/destructor for the bommon OutputFormatter base class.
On 2015/12/01 22:13:30, jeffbrown wrote:
> common

Done.

[johngro, 2016-01-28 22:17:13]

Description was changed from

==========
Mix to an intermediate buffer.

Mix audio into an intermediate buffer of 16 bit signed integers stored
in 32 bit values.  Do not clip and convert to the final output format
until all of the input tracks have been accumulated into the
intermediate mix buffer.

R=dalesat@chromium.org, jeffbrown@google.com
BUG=
==========

to

==========
Mix to an intermediate buffer.

Mix audio into an intermediate buffer of 16 bit signed integers stored
in 32 bit values.  Do not clip and convert to the final output format
until all of the input tracks have been accumulated into the
intermediate mix buffer.

R=jeffbrown@google.com, dalesat@chromium.org
BUG=

Committed:
https://chromium.googlesource.com/external/mojo/+/8d428b829a8b5efbe7f4bf33a32...
==========

[johngro, 2016-01-28 22:17:15]

Committed patchset #4 (id:60001) manually as
8d428b829a8b5efbe7f4bf33a329a3ae7226958c (presubmit successful).