1262746 - Add downmix capabilities to AudioConverter

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Jean-Yves Avenard [:jya]]

This is to track the task of adding dowmixing capabilities to the downmixer.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: MozReview Request: Bug 1262746: P1. Add downmixing capabilities to AudioConverter. r?rillian

This is using the same downmixer algorithm as found in the ogg reader.

Review commit: https://reviewboard.mozilla.org/r/45195/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45195/

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: MozReview Request: Bug 1262746: P2. Use AudioConvert in AudioStream to downmix samples. r?jwwang

Review commit: https://reviewboard.mozilla.org/r/45197/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45197/

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: MozReview Request: Bug 1262746: P3. Remove DownmixAudioToStereo method. r?rillian

Functionality is now provided through AudioConverter class.

Review commit: https://reviewboard.mozilla.org/r/45199/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45199/

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8739328 [details]
MozReview Request: Bug 1262746: P2. Use AudioConvert in AudioStream to downmix samples. r?jwwang

https://reviewboard.mozilla.org/r/45197/#review41707

::: dom/media/AudioStream.cpp:358
(Diff revision 1)
>    params.format = ToCubebFormat<AUDIO_OUTPUT_FORMAT>::value;
>    mAudioClock.Init();
>  
> +  mInConfig = MakeUnique<AudioConfig>(mChannels, mInRate);
> +  mOutConfig = MakeUnique<AudioConfig>(mOutChannels, mOutRate);
> +  mAudioConverter = MakeUnique<AudioConverter>(*mInConfig.get(), *mOutConfig.get());

You can say |MakeUnique<>(*mInConfig, *mOutConfig)|. UniquePtr has operation*.

Comment 5

[Ralph Giles (:rillian)]

Comment on attachment 8739327 [details]
MozReview Request: Bug 1262746: P1. Add downmixing capabilities to AudioConverter. r?rillian

https://reviewboard.mozilla.org/r/45195/#review42085

Sorry it took me so long to look at this. I didn't check the coefficients again. I assume they're cut-and-pasted from the updates in the earlier patches.

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 8739329 [details]
MozReview Request: Bug 1262746: P3. Remove DownmixAudioToStereo method. r?rillian

https://reviewboard.mozilla.org/r/45199/#review42087

...other times I just forget to hit the three-click publish button sequence.

If you export the clip function, you can remove MOZ_CLIP_TO_15() from VideoUtils.h and replace the other calls (OggReader, OpusDecoder) with AudioConverter::clipTo15().

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8739327 [details]
MozReview Request: Bug 1262746: P1. Add downmixing capabilities to AudioConverter. r?rillian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45195/diff/1-2/

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8739328 [details]
MozReview Request: Bug 1262746: P2. Use AudioConvert in AudioStream to downmix samples. r?jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45197/diff/1-2/

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8739329 [details]
MozReview Request: Bug 1262746: P3. Remove DownmixAudioToStereo method. r?rillian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45199/diff/1-2/

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b77cdb6c0b6c
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b3a0fdc1e61
https://hg.mozilla.org/integration/mozilla-inbound/rev/3955e76756ce

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b77cdb6c0b6c
https://hg.mozilla.org/mozilla-central/rev/5b3a0fdc1e61
https://hg.mozilla.org/mozilla-central/rev/3955e76756ce