Closed Bug 973362 Opened 6 years ago Closed 6 years ago

Enable debug assertions for libopus

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla30
Tracking Flags:
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

Details

Attachments

(1 file)

enable-libopus-assertions.patch
1.28 KB, patch
derf
: review+
Details | Diff | Splinter Review 
Firefox does not enable debug assertions for some third-party libraries. libpous is one of those libraries. We must #define ENABLE_ASSERTIONS, not just DEBUG, to enable celt_assert and silk_assert.
Attachment #8376852 - Flags: review?(tterribe)
Comment on attachment 8376852 [details] [diff] [review]
enable-libopus-assertions.patch

Review of attachment 8376852 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, but see the comment. You really only want the first hunk.

::: media/libopus/moz.build
@@ +38,5 @@
>      DEFINES['HAVE_ALLOCA_H'] = True
>  
>  if not CONFIG['MOZ_SAMPLE_TYPE_FLOAT32']:
>      DEFINES['FIXED_POINT'] = 1
> +    DEFINES['FIXED_DEBUG_ASSERT'] = True

This wraps almost every basic arithmetic operation in a function call with input/ouput checks. That's probably not something you want on by default, even in debug builds. Even the upstream developers only enable them to track down specific problems.
Attachment #8376852 - Flags: review?(tterribe) → review+
cdiehl, please fuzz opus in debug builds again once this lands :)
https://hg.mozilla.org/mozilla-central/rev/143179ce1dad
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.