Closed
Bug 847827
Opened 12 years ago
Closed 11 years ago
Update the in-tree copy of libsoundtouch to get multichannel support
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: padenot, Assigned: michael)
Details
(Whiteboard: [mentor=padenot][lang=c++])
Attachments
(4 files, 3 obsolete files)
12.72 KB,
patch
|
Details | Diff | Splinter Review | |
16.50 KB,
patch
|
Details | Diff | Splinter Review | |
57.54 KB,
patch
|
Details | Diff | Splinter Review | |
114.67 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
We are using the SoundTouch library to implement the |playbackRate| member of the <audio> and <video> tags. This library gives us the ability to time stretch the audio, so it plays slower or faster, without affecting the pitch (so we don't get, say, a mickey mouse voice when playing the media at high speed).
SoundTouch only supports up to two channels (mono or stereo audio), and because we can play media file that have more than two channels, we need to either:
(1) Have multiple time stretcher in the AudioStream instance to handle all the channels (bad way)
(2) Patch SoundTouch so it can handle more than two channels, and upstream the patches (good way)
This bug is about implementing the second solution.
The code is in [1], and the library is independent [2], so there is not too much code to read before actually implementing the feature.
[1]: http://mxr.mozilla.org/mozilla-central/source/media/libsoundtouch/
[2]: http://www.surina.net/soundtouch/
Comment 1•12 years ago
|
||
Hi Paul,
I am just starting out and I would like to work on this ....
I will go through the docs and get in touch.
Ajay
Reporter | ||
Comment 2•12 years ago
|
||
Ajay, if you need info, come on IRC, on irc.mozilla.com, channel #media, during late European timezone (UTC+1), I'm "padenot" there.
Comment 3•12 years ago
|
||
Hi Paul,
I read the link related to Soundtouch library and some of the stuff related to mono and stereo sounds.
Could you provide me some more related details :
1) This is related to Firefox OS ?
2) could I see the working of this sound library somewhere and its failure in case when channels > 2 ?
3) Any other docs which I should read ?
(I came to the #media channel I just tried to contact you on the normal chat, should I PM you next time)
Thanks;
Ajay
Reporter | ||
Comment 4•12 years ago
|
||
1) It is not related to Firefox OS right now, the |playbackRate| member of the audio and video elements have not made up in time when the Firefox OS branch was cut. However, it will be in the second Firefox OS version for sure. So that leaves us with Firefox Desktop and Firefox for Android for now.
2a) Inside of Firefox, just get a nightly build (http://nightly.mozilla.org) for your platform, go and play any HTML5 video (such as those on this test page: http://paul.cx/public/playbackrate/, there are buttons to change the playback rate. Note that the pitch is not affected. The playback rate modification is achieved using the SoundTouch library. Inspecting the code, you see that this is achieved by setting the |playbackRate| property of an audio or video element).
2b) Outside of Firefox, simply get the source code of the library, compile it (there is instructions in the package), and run the examples. It might be easier working with the stand alone library.
2c) The failure case is simply an assert: http://mxr.mozilla.org/mozilla-central/source/media/libsoundtouch/src/SoundTouch.cpp#146
3) I believe reading the source code of SoundTouch will help. Basically, all we need to do here is to make every piece of code that is mono or stereo specific able to handle an arbitrary number of channels.
I saw you tried to ping me on irc, but I was not in front of my computer. Contacting me in the channel is fine :-).
Comment 5•12 years ago
|
||
I will check this info and will conatct you
Comment 6•12 years ago
|
||
Hi Paul,
I was trying out some changes in the soundtouch libraray files (.cpp files)
Basically I am using SoundStretch tool provided with it on a sample .wav file
I am trying to add some printfs or cout inside files like SoundTouch.cpp, but the output doesnot come on screen or in dmesg (Althogh it works without any error)? Any ideas how can I add printfs inside the Soundtocuh library?
Ajay
Comment 7•11 years ago
|
||
I just changed the files for FIRFilter. There could have been similar changes to the files RateTransposer.cpp and TDStretch.cpp but it seems that they set the channel to 2 in the constructor, so I did not touch them. Should they be changed as well?
Attachment #760995 -
Flags: feedback?(paul)
Comment 8•11 years ago
|
||
Reporter | ||
Comment 9•11 years ago
|
||
So, the maintainer of libsoundtouch told us today that someone just added multichannel support to the library. This bug is now about importing the new version of the library and updating the patch we have on top, making sure it works properly, writing a test that uses a multichannel file and uses the library.
Assignee: nobody → peyrard.max43
Summary: Make the SoundTouch library able to handle more than 2 channels. → Update the in-tree copy of libsoundtouch to get multichannel support
Reporter | ||
Comment 10•11 years ago
|
||
Comment on attachment 760995 [details] [diff] [review]
patch v1
Unsetting the flag since this is not needed anymore.
Attachment #760995 -
Flags: feedback?(paul)
Comment 11•11 years ago
|
||
Attachment #764048 -
Flags: review?(paul)
Reporter | ||
Comment 12•11 years ago
|
||
Comment on attachment 764048 [details] [diff] [review]
This patch updates the soundtouch library and adds a test
Review of attachment 764048 [details] [diff] [review]:
-----------------------------------------------------------------
You forgot to hg add the test file!
Attachment #764048 -
Flags: review?(paul)
Comment 13•11 years ago
|
||
Attachment #764048 -
Attachment is obsolete: true
Attachment #764061 -
Flags: review?(paul)
Reporter | ||
Updated•11 years ago
|
Attachment #764061 -
Flags: review?(paul) → review+
Reporter | ||
Comment 14•11 years ago
|
||
Looks good. Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=2ceb1050ddb2
Comment 15•11 years ago
|
||
Attachment #764061 -
Attachment is obsolete: true
Attachment #764137 -
Flags: review?(paul)
Reporter | ||
Comment 16•11 years ago
|
||
Comment on attachment 764137 [details] [diff] [review]
Patch updated : alloca function replaced by malloc
Review of attachment 764137 [details] [diff] [review]:
-----------------------------------------------------------------
We found that this version is super crashy, cancelling review for now.
Attachment #764137 -
Flags: review?(paul)
Comment 17•11 years ago
|
||
A new version of libsoundtouch was released in Jan-2014. Should we reconsider this?
Reporter | ||
Comment 18•11 years ago
|
||
We should, yes.
Comment 19•11 years ago
|
||
Let me email the core developer of libsoundtouch and I will report here.
Comment 20•11 years ago
|
||
Core developer of libsoundtouch replied to my email:
"The multichannel support has been there quite some time now, and no bug reports have been submitted about it for ages so I consider it as stable :).
For testing and debugging reasons, in case you're interested about such, there's a compile-time flag which enforces using multi-channel routine always also for mono- and stereo-sound, because 1- and 2-channel audio can be considered as specific cases of multichannel audio.
Benefit of such multichannel-algorithm-enforce mode is to achieve better testing exposure for the multichannel support (as most available audio sources do not have more than 2 channels), with penalty of bit worse runtime performance as optimizations of native mono/stereo audio routines then do not apply, yet such is quite a small issue nowadays anyway.
best regards, Olli"
Assignee | ||
Comment 21•11 years ago
|
||
I've updated libsoundtouch to r198, which includes support for multichannel audio. I have also updated AudioStream.cpp and MediaDecoderStateMachine.cpp in content/media to remove the restriction on multichannel audio sources. Finally I have added a 6-channel audio file (content/media/test/bug495794.ogg) to the list of files to be used for testing playback.
Assignee | ||
Updated•11 years ago
|
Attachment #8403053 -
Flags: feedback?(paul)
Reporter | ||
Comment 22•11 years ago
|
||
Comment on attachment 8403053 [details] [diff] [review]
update-libsoundtouch-multichannel.patch
Review of attachment 8403053 [details] [diff] [review]:
-----------------------------------------------------------------
http://tbpl.mozilla.org/?tree=Try&rev=50493f2f4584
::: content/media/test/manifest.js
@@ +44,5 @@
> { name:"seek.webm", type:"video/webm", duration:3.966 },
> { name:"gizmo.mp4", type:"video/mp4", duration:5.56 },
> { name:"owl.mp3", type:"audio/mpeg", duration:3.29 },
> + { name:"vbr.mp3", type:"audio/mpeg", duration:10.0 },
> + { name:"bug495794.ogg", type:"audio/ogg", duration:0.3 }
Note that the tests that currently exercise .playbackRate are currently disabled. I'll push to try servers with them enabled.
Attachment #8403053 -
Flags: feedback?(paul) → feedback+
Reporter | ||
Comment 23•11 years ago
|
||
We need to do something for Windows, it does not have alloca, but has _alloca. A little #define hackery will work, I think.
Reporter | ||
Comment 24•11 years ago
|
||
Comment on attachment 8403053 [details] [diff] [review]
update-libsoundtouch-multichannel.patch
Review of attachment 8403053 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/libsoundtouch/src/FIRFilter.cpp
@@ +169,5 @@
>
> +uint FIRFilter::evaluateFilterMulti(SAMPLETYPE *dest, const SAMPLETYPE *src, uint numSamples, uint numChannels) const
> +{
> + uint i, j, end, c;
> + LONG_SAMPLETYPE *sum=(LONG_SAMPLETYPE*)alloca(numChannels*sizeof(*sum));
Here, this does not compile on vs2010, which is our default compiler on windows.
::: media/libsoundtouch/src/SoundTouch.cpp
@@ +348,4 @@
> int i;
> int nUnprocessed;
> int nOut;
> + SAMPLETYPE *buff=(SAMPLETYPE*)alloca(64*channels*sizeof(SAMPLETYPE));
Also there.
Assignee | ||
Comment 25•11 years ago
|
||
I've updated FIRFilter.cpp and SoundTouch.cpp to use _alloca when compiling with MSVC.
Attachment #8403053 -
Attachment is obsolete: true
Attachment #8403373 -
Flags: feedback?(paul)
Reporter | ||
Comment 26•11 years ago
|
||
Comment on attachment 8403373 [details] [diff] [review]
update-libsoundtouch-multichannel.patch
Review of attachment 8403373 [details] [diff] [review]:
-----------------------------------------------------------------
Can you please regenerate the file moz-libsoundtouch.patch, now that we carry additional changes (alloca -> _alloca) ?
Attachment #8403373 -
Flags: feedback?(paul)
Reporter | ||
Comment 27•11 years ago
|
||
Forget that. I pushed to try again, it looks good: https://tbpl.mozilla.org/?tree=Try&rev=28b9150c2cb3
Assignee | ||
Updated•11 years ago
|
Attachment #8403373 -
Flags: review?(paul)
Reporter | ||
Updated•11 years ago
|
Attachment #8403373 -
Flags: review?(paul) → review+
Reporter | ||
Comment 28•11 years ago
|
||
Assignee: peyrard.max43 → michael
Comment 29•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•