This results in a speed improvement of about 6% on the "Convolution reverb" webaudio-benchmark (measured with Linux x86_64 build on Skylake), and should mean less code to ship.

Comment on attachment 8992577 [details] bug 1476231 remove unnecessary x86/cpuid.s, which does not exist in libav-11.3 https://reviewboard.mozilla.org/r/257438/#review264362

Comment on attachment 8992578 [details] bug 1476231 add ffmpeg floating point real FFT functions to ffvpx when MOZ_LIBAV_FFT is configured https://reviewboard.mozilla.org/r/257440/#review264364

Comment on attachment 8992580 [details] bug 1476231 add accessor for ffvpx real DFT functions https://reviewboard.mozilla.org/r/257444/#review264368 ::: dom/media/platforms/ffmpeg/FFmpegLibWrapper.h:10 (Diff revision 1) > #ifndef __FFmpegLibWrapper_h__ > #define __FFmpegLibWrapper_h__ > > #include "mozilla/Attributes.h" > #include "mozilla/Types.h" > +#include "FFVPXRuntimeLinker.h" // for enum RDFTransformType not sure I like the way it's organised in there... and the apparent cycle in header loading. Can we use a separate header for those two? that would be included as required. ::: dom/media/platforms/ffmpeg/ffvpx/FFVPXRuntimeLinker.cpp:125 (Diff revision 1) > } > return FFmpegDecoderModule<FFVPX_VERSION>::Create(&sFFVPXLib); > } > > +/* static */ void > +FFVPXRuntimeLinker::GetRDFTFuncs(FFVPXRuntimeLinker::RDFTFuncs* aOutFuncs) FFVPXRuntimeLinker::GetRDFTFuncs(RDFTFuncs* aOutFuncs)

Comment on attachment 8992581 [details] bug 1476231 switch FFTBlock from libav to ffvpx https://reviewboard.mozilla.org/r/257446/#review264370 ::: dom/media/webaudio/FFTBlock.h:305 (Diff revision 1) > #endif > > void Clear() > { > #if defined(MOZ_LIBAV_FFT) > - av_rdft_end(mAvRDFT); > + if (av_rdft.end) { testing if (mAVRFDT || mAVIRDFT) seems more obvious to me, av_rfdt.end can then be in MOZ_ASSERT instead ::: dom/media/webaudio/FFTBlock.h:325 (Diff revision 1) > } > void AddConstantGroupDelay(double sampleFrameDelay); > void InterpolateFrequencyComponents(const FFTBlock& block0, > const FFTBlock& block1, double interp); > #if defined(MOZ_LIBAV_FFT) > + static FFVPXRuntimeLinker::RDFTFuncs av_rdft; Can we use a mozilla-style name here? makes it confusing reading the code ,as you don't know if we're accessing a ffmpeg function or what... maybe mAvRfdtLib

Comment on attachment 8992582 [details] bug 1476231 remove now-unused libav from lgpllibs https://reviewboard.mozilla.org/r/257448/#review264372 \o/

Comment on attachment 8992579 [details] bug 1476231 link ffmpeg real DFT functions https://reviewboard.mozilla.org/r/257442/#review264374 ::: dom/media/platforms/ffmpeg/FFmpegLibWrapper.h:19 (Diff revision 1) > struct AVPacket; > struct AVDictionary; > struct AVCodecParserContext; > struct PRLibrary; > > +struct RDFTContext; see comment in patch 4

Comment on attachment 8992580 [details] bug 1476231 add accessor for ffvpx real DFT functions https://reviewboard.mozilla.org/r/257444/#review264368 > FFVPXRuntimeLinker::GetRDFTFuncs(RDFTFuncs* aOutFuncs) This is different now with included as required.

Comment on attachment 8992581 [details] bug 1476231 switch FFTBlock from libav to ffvpx https://reviewboard.mozilla.org/r/257446/#review264370 > testing if (mAVRFDT || mAVIRDFT) seems more obvious to me, av_rfdt.end can then be in MOZ_ASSERT instead Switched to testing mAVRFDT or mAVIRDFT as appropriate. No need to fill around the code with asserts, because we'll know if the function pointer is null. > Can we use a mozilla-style name here? > > makes it confusing reading the code ,as you don't know if we're accessing a ffmpeg function or what... > > maybe mAvRfdtLib It's now sRDFTFuncs.

Comment on attachment 8992580 [details] bug 1476231 add accessor for ffvpx real DFT functions https://reviewboard.mozilla.org/r/257444/#review267936 ::: dom/media/platforms/moz.build:51 (Diff revision 3) > + EXPORTS += [ > + 'ffmpeg/FFmpegRDFTTypes.h', > + ] dom/media/platforms/ffmpeg/moz.build is used only when MOZ_FFMPEG is defined, and MOZ_FFMPEG is not defined on NT, so the export is moved to the same moz.build file as associated code.

Comment on attachment 8992581 [details] bug 1476231 switch FFTBlock from libav to ffvpx https://reviewboard.mozilla.org/r/257446/#review267940 ::: testing/web-platform/meta/webaudio/the-audio-api/the-convolvernode-interface/convolver-response-1-chan.html.ini:2 (Diff revision 3) > [convolver-response-1-chan.html] > - [X 1: Channel 1: Expected 0 for all values but found 1280 unexpected values: \n\tIndex\tActual\n\t[0\]\t-1.4901161193847656e-7\n\t[1\]\t-8.940696716308594e-8\n\t[2\]\t0.3311062455177307\n\t[3\]\t0.6248594522476196\n\t...and 1276 more errors.] > + [X 1: Channel 1: Expected 0 for all values but found 1280 unexpected values: \n\tIndex\tActual\n\t[0\]\t-1.1920928955078125e-7\n\t[1\]\t-4.470348358154297e-8\n\t[2\]\t0.3311062455177307\n\t[3\]\t0.6248593926429749\n\t...and 1276 more errors.] Numerical differences show up here due to bug 1474222 and https://github.com/web-platform-tests/wpt/issues/10201. ::: testing/web-platform/meta/webaudio/the-audio-api/the-convolvernode-interface/convolver-response-2-chan.html.ini:17 (Diff revision 3) > expected: FAIL > > [# AUDIT TASK RUNNER FINISHED: 2 out of 6 tasks were failed.] > expected: FAIL > > + [X 1: Channel 0 does not equal [0,0,0.9458408951759338,0.8448333740234375,0.8210252523422241,0.8620985746383667,0.8430315852165222,0.855602502822876,0.7933436632156372,0.9865825176239014,0.3972480297088623,-0.7786127924919128,-0.9223549962043762,-0.7896472215652466,-0.8727429509162903,-0.8325281143188477...\] with an element-wise tolerance of {"absoluteThreshold":3.5763e-7,"relativeThreshold":0}.\n\tIndex\tActual\t\t\tExpected\t\tAbsError\t\tRelError\t\tTest threshold\n\t[267\]\t8.6412906646728516e-1\t8.6412948369979858e-1\t4.1723251342773438e-7\t4.8283564129919487e-7\t3.5763000000000001e-7\n\t[779\]\t-8.6412906646728516e-1\t-8.6412948369979858e-1\t4.1723251342773438e-7\t4.8283564129919487e-7\t3.5763000000000001e-7\n\tMax AbsError of 4.1723251342773438e-7 at index of 267.\n\tMax RelError of 4.8283564129919487e-7 at index of 267.\n] The multiple tolerances in this test were tightly tuned for Firefox with libav. On linux32, where libav was not used, the subtests where numerical errors were the largest differed, and so there are already expected failures. The switch to ffmpeg similarly changes the particular subtests were numerical errors are largest, and so some more subtests are classed as failing. This is bug 1475158, the fix for which is waiting on review. ::: testing/web-platform/meta/webaudio/the-audio-api/the-convolvernode-interface/convolver-response-4-chan.html.ini:48 (Diff revision 3) > expected: FAIL > > [< [5.1-channel input\] 2 out of 2 assertions were failed.] > expected: FAIL > > - [X 1: Channel 0 expected to be equal to the array [0,0,0.9458408951759338,0.8448333740234375,1.7668662071228027,1.7069319486618042,1.6640567779541016,1.7177010774612427,1.6363751888275146,1.8421850204467773,1.1905916929244995,0.20796972513198853,-0.5251069664955139,-1.5682599544525146,-1.7950979471206665,-1.6221753358840942...\] but differs in 965 places:\n\tIndex\tActual\t\t\tExpected\n\t[0\]\t-2.9802322387695313e-8\t0.0000000000000000e+0\n\t[1\]\t-7.4505805969238281e-8\t0.0000000000000000e+0\n\t[2\]\t9.4584065675735474e-1\t9.4584089517593384e-1\n\t[4\]\t1.7668659687042236e+0\t1.7668662071228027e+0\n\t...and 961 more errors.] > + [X 1: Channel 0 expected to be equal to the array [0,0,0.9458408951759338,0.8448333740234375,1.7668662071228027,1.7069319486618042,1.6640567779541016,1.7177010774612427,1.6363751888275146,1.8421850204467773,1.1905916929244995,0.20796972513198853,-0.5251069664955139,-1.5682599544525146,-1.7950979471206665,-1.6221753358840942...\] but differs in 969 places:\n\tIndex\tActual\t\t\tExpected\n\t[0\]\t1.1920928955078125e-7\t0.0000000000000000e+0\n\t[1\]\t-7.4505805969238281e-8\t0.0000000000000000e+0\n\t[2\]\t9.4584071636199951e-1\t9.4584089517593384e-1\n\t[3\]\t8.4483325481414795e-1\t8.4483337402343750e-1\n\t...and 965 more errors.] This is also bug 1475158, the fix for which is waiting on review.

Pushed by ktomlinson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/056ccdf7f059 remove unnecessary x86/cpuid.s, which does not exist in libav-11.3 r=jya https://hg.mozilla.org/integration/autoland/rev/08a8351aca6f add ffmpeg floating point real FFT functions to ffvpx when MOZ_LIBAV_FFT is configured r=jya https://hg.mozilla.org/integration/autoland/rev/fefcc2674976 link ffmpeg real DFT functions r=jya https://hg.mozilla.org/integration/autoland/rev/c988d5a322d3 add accessor for ffvpx real DFT functions r=jya https://hg.mozilla.org/integration/autoland/rev/0fbbfd857bb8 switch FFTBlock from libav to ffvpx r=jya https://hg.mozilla.org/integration/autoland/rev/beddb157da49 remove now-unused libav from lgpllibs r=jya

https://hg.mozilla.org/mozilla-central/rev/056ccdf7f059 https://hg.mozilla.org/mozilla-central/rev/08a8351aca6f https://hg.mozilla.org/mozilla-central/rev/fefcc2674976 https://hg.mozilla.org/mozilla-central/rev/c988d5a322d3 https://hg.mozilla.org/mozilla-central/rev/0fbbfd857bb8 https://hg.mozilla.org/mozilla-central/rev/beddb157da49