Convert debug-only asserts added in bug 1303247 to release asserts

RESOLVED FIXED in Firefox 52

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

Trunk
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
I used MOZ_ASSERT (which are only fatal in debug builds) when I really wanted MOZ_DIAGNOSTIC_ASSERT (which are fatal in debug and non-debug builds, but non-fatal in release builds).  The intention is to have these asserts (when enabled with the media.rust.test-mode pref) fatal in regular nightly builds to make it easier to catch Rust/Stagefright mismatches while dogfooding.
(Assignee)

Comment 1

a year ago
Created attachment 8795086 [details] [diff] [review]
bug1305596_v0.patch

Also converts the two commented out AudioInfo checks into warnings.
Attachment #8795086 - Flags: review?(giles)
Comment on attachment 8795086 [details] [diff] [review]
bug1305596_v0.patch

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

::: media/libstagefright/binding/MP4Metadata.cpp
@@ +298,5 @@
> +      MOZ_DIAGNOSTIC_ASSERT(audioRust->mRate == audio->mRate);
> +      MOZ_DIAGNOSTIC_ASSERT(audioRust->mChannels == audio->mChannels);
> +      MOZ_DIAGNOSTIC_ASSERT(audioRust->mBitDepth == audio->mBitDepth);
> +      NS_WARN_IF(audioRust->mProfile != audio->mProfile);
> +      NS_WARN_IF(audioRust->mExtendedProfile != audio->mExtendedProfile);

nsDebug.h says this is MOZ_MUST_USE in debug builds, and one must prepend `UNUSED <<` to use it this way.
Attachment #8795086 - Flags: review?(giles) → review+
(Assignee)

Comment 3

a year ago
(In reply to Ralph Giles (:rillian) needinfo me from comment #2)
> nsDebug.h says this is MOZ_MUST_USE in debug builds, and one must prepend
> `UNUSED <<` to use it this way.

Ah, thanks.  I'll just leave them commented out instead, since I didn't realize NS_WARN_IF also asserts in debug builds, which isn't useful in this case.

Comment 4

a year ago
Pushed by mgregan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9429b90c5754
Convert debug-only asserts added in bug 1303247 to release asserts.  r=rillian

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9429b90c5754
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.