Closed Bug 1305596 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: kinetik, Assigned: kinetik)

References

Details

Attachments

(1 file)

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.
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+
(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.
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
https://hg.mozilla.org/mozilla-central/rev/9429b90c5754
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.