Closed Bug 1372767 Opened 5 years ago Closed 5 years ago

opus: Enable SSE optimization

Categories

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

Product:
Core
Component:
Audio/Video: Playback
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: rillian, Assigned: rillian)

References

Details

Attachments

(1 file)

Bug 1372767 - opus: enable SSE simd optimization.
59 bytes, text/x-review-board-request
kinetik
: review+
ted
: review+
Details 
Opus has gained sse 4.1 simd optimizations. We should enable that in our builds.
Assignee: nobody → giles
Comment on attachment 8877367 [details]
Bug 1372767 - opus: enable SSE simd optimization.

https://reviewboard.mozilla.org/r/148756/#review153194

::: media/libopus/moz.build:89
(Diff revision 1)
>      ]
>      UNIFIED_SOURCES += silk_sources_fixed
> -# for webrtc
> -    UNIFIED_SOURCES += opus_sources_float
>  
> +if CONFIG['CPU_ARCH'] in ('x86', '86_64'):

86_64 should be x86_64 I assume
Attachment #8877367 - Flags: review?(kinetik) → review+
Comment on attachment 8877367 [details]
Bug 1372767 - opus: enable SSE simd optimization.

https://reviewboard.mozilla.org/r/148756/#review153566

::: media/libopus/moz.build:89
(Diff revision 1)
>      ]
>      UNIFIED_SOURCES += silk_sources_fixed
> -# for webrtc
> -    UNIFIED_SOURCES += opus_sources_float
>  
> +if CONFIG['CPU_ARCH'] in ('x86', '86_64'):

Indeed it should! Thanks. I thought that went a little too smoothly...
Comment on attachment 8877367 [details]
Bug 1372767 - opus: enable SSE simd optimization.

https://reviewboard.mozilla.org/r/148756/#review153596
Attachment #8877367 - Flags: review?(ted) → review+
Comment on attachment 8877367 [details]
Bug 1372767 - opus: enable SSE simd optimization.

https://reviewboard.mozilla.org/r/148756/#review153636

::: media/libopus/moz.build:95
(Diff revision 1)
> +    DEFINES['OPUS_HAVE_RTCD'] = True
> +    DEFINES['OPUS_X86_MAY_HAVE_SSE'] = True
> +    DEFINES['OPUS_X86_MAY_HAVE_SSE2'] = True
> +    DEFINES['OPUS_X86_MAY_HAVE_SSE4_1'] = True
> +    DEFINES['OPUS_X86_MAY_HAVE_AVX'] = True
> +    UNIFIED_SOURCES += celt_sources_sse

i686-linux still builds with -mmmx by default so we need to set -msse for these files.

::: media/libopus/moz.build:105
(Diff revision 1)
> +    else:
> +        UNIFIED_SOURCES += silk_sources_fixed_sse4_1
> +    if not CONFIG['_MSC_VER'] or CONFIG['CLANG_CL']:
> +        for f in UNIFIED_SOURCES:
> +            if f in celt_sources_sse2:
> +                UNIFIED_SOURCES[f].flags += CONFIG['SSE2_FLAGS']

I think we have to use `SOURCES` here to attach individual tags.
Comment on attachment 8877367 [details]
Bug 1372767 - opus: enable SSE simd optimization.

https://reviewboard.mozilla.org/r/148756/#review153666

Updated to address issues from try.
Comment on attachment 8877367 [details]
Bug 1372767 - opus: enable SSE simd optimization.

https://reviewboard.mozilla.org/r/148756/#review154076

::: media/libopus/moz.build:99
(Diff revision 2)
> +    DEFINES['OPUS_X86_MAY_HAVE_AVX'] = True
> +    SOURCES += celt_sources_sse
> +    SOURCES += celt_sources_sse2
> +    SOURCES += celt_sources_sse4_1
> +    if CONFIG['MOZ_SAMPLE_TYPE_FLOAT32']:
> +        SOURCES += silk_sources_sse4_1

Turns out `silk_sources_sse4_1` should be included unconditionally; it's shared between the fixed and float variants. Only `silk_sources_fixed_sse4_1` needs a conditional here.
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4175246fd7a1
opus: enable SSE simd optimization. r=kinetik,ted
https://hg.mozilla.org/mozilla-central/rev/4175246fd7a1
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
one compiler warning alert (for information only, no action required):
== Change summary for alert #7335 (as of June 15 2017 19:53 UTC) ==

Regressions:

  3%  compiler warnings summary osx-10-7-noopt debug      6,542.33 -> 6,770.83

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7335
You need to log in before you can comment on or make changes to this bug.