Closed Bug 1262746 Opened 4 years ago Closed 4 years ago

Add downmix capabilities to AudioConverter

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

Attachments

(3 files)

This is to track the task of adding dowmixing capabilities to the downmixer.
Blocks: 1247138
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/
Attachment #8739327 - Flags: review?(giles)
Attachment #8739328 - Flags: review?(jwwang)
Attachment #8739329 - Flags: review?(giles)
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*.
Attachment #8739328 - Flags: review?(jwwang) → review+
Attachment #8739327 - Flags: review?(giles) → review+
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.
Attachment #8739329 - Flags: review?(giles) → review+
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 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 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 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/
You need to log in before you can comment on or make changes to this bug.