Closed Bug 1378507 Opened 3 years ago Closed 3 years ago

Crash in InvalidArrayIndex_CRASH | nsTArray_Impl<T>::ElementAt | mp4_demuxer::H264::ExtractExtraData

Categories

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

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 - fixed
firefox56 --- fixed

People

(Reporter: mccr8, Assigned: jya)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-7dd0f191-82e0-4908-adda-a92f90170705.
=============================================================

Index is 4 in all of these crashes, and the array length is 0.

20 of these crashes, all on Nightly, starting on the 6-29 build.
Flags: needinfo?(continuation)
This would normally be caught with the MOZ_ASSERT(AnnexB::IsAVCC(aSample)); above.

How we get there with non AVCC sample is beyond me at this stage.
What is the needinfo for? I came across this on crash stats. Looking over the crash reports, it is possible this is just one user, so maybe something is just weird with their configuration or machine. Feel free to close this if you think it isn't actionable.
Flags: needinfo?(continuation)
Assignee: nobody → jyavenard
Blocks: 1313398
VP9 in MP4 would definitely cause this to crash.
This isn't a valid webm, it has a h264 track.

Per spec, webm is to contain only opus, vorbis, vp8, vp9 (and now av1)
http://www.webmproject.org/docs/container/
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
wrong bug
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Comment on attachment 8883857 [details]
Bug 1378507: Add extra check to ensure data is valid.

https://reviewboard.mozilla.org/r/154808/#review160084

::: commit-message-835c7:1
(Diff revision 1)
> +Bug 1378507: Add extra check to ensure data is valid. r?gerald

Traditionally commit messages start with "Bug 1378507 - Add ...", i.e., dash instead of colon.
Attachment #8883857 - Flags: review?(gsquelart) → review+
Comment on attachment 8883857 [details]
Bug 1378507: Add extra check to ensure data is valid.

https://reviewboard.mozilla.org/r/154808/#review160084

> Traditionally commit messages start with "Bug 1378507 - Add ...", i.e., dash instead of colon.

what the... you're only telling me this now?

I my 1800ish history of commits, not once have I used a dash next to the bug number :(

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities
only states that the commit message "should include the bug number, the names of the reviewers, and a clear explanation of the fix."

Will change from now on
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/951d6b30f1f6
Add extra check to ensure data is valid. r=gerald
https://hg.mozilla.org/mozilla-central/rev/951d6b30f1f6
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Untracked and 55 unaffected as I don't see any crash signatures on 55 in the past one month. Please let me know if I misread the data.
Blocks: 1374774
Comment on attachment 8883857 [details]
Bug 1378507: Add extra check to ensure data is valid.

Approval Request Comment
[Feature/Bug causing the regression]: 1374774
[User impact if declined]: crashes. Bug 1374774 is about to be uplifted, this change must go in to avoid the downfall.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: hard to say..
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: adding extra check to avoid entering crash condition
[String changes made/needed]: none
Attachment #8883857 - Flags: approval-mozilla-beta?
Comment on attachment 8883857 [details]
Bug 1378507: Add extra check to ensure data is valid.

regression from 1374774, they should go in beta together

I guess you're going to rebase the patches from 1374774?
Flags: needinfo?(jyavenard)
Attachment #8883857 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Yes. I have a patch stack ready to be pushed. Was just waiting on this r+
Flags: needinfo?(jyavenard)
You need to log in before you can comment on or make changes to this bug.