Closed Bug 1219047 Opened 8 years ago Closed 8 years ago

Call rust mp4parser from MP4Metadata


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

Not set



Tracking Status
firefox44 --- affected
firefox45 --- fixed


(Reporter: rillian, Assigned: rillian)




(1 file, 2 obsolete files)

Anthony tells me sooner is better than later, so we should land my hacked-up buffer-copy patch calling the rust parser on init segments as soon as it's safe.
Attached patch Call rust mp4 parser v1 (obsolete) — Splinter Review
Here's my WIP hack. Needs conditionals, and seems to crash on quit.

Anthony, you can apply this and compile with `ac_add_options --enable-rust` in your mozconfig. Then:

./mach run bruce_vs_ironman_frag.mp4
rust parser found 2 tracks
Flags: needinfo?(ajones)
Depends on: 1219530
Pushed a version with MOZ_RUST conditionals (needs bug 1219530) to try:

I don't see the shutdown crash on Linux x64 debug with rustc 1.3.0... it'd be good to track that down.
The crash happened when I killed firefox (ctrl-C on the terminal) while a video was playing. Doesn't seem to happen if I quit from the gui.
Depends on: 1220754
Depends on: 1219983
Attached patch Call rust mp4 parser v2 (obsolete) — Splinter Review
Let's get something landed.
Attachment #8679721 - Attachment is obsolete: true
Attachment #8682104 - Flags: review?(kinetik)
Comment on attachment 8682104 [details] [diff] [review]
Call rust mp4 parser v2

Review of attachment 8682104 [details] [diff] [review]:

r+ with assert converted to an error return.

::: media/libstagefright/binding/MP4Metadata.cpp
@@ +118,5 @@
> +    fprintf(stderr, "Couldn't get source length\n");
> +    return false;
> +  }
> +  fprintf(stderr, "Source length %d bytes\n", (long long int)length);
> +  MOZ_ASSERT(length > 0);

My earlier try run showed this failing; Length() can return -1 for unknown stream lengths I guess.
Attachment #8682104 - Flags: review?(kinetik) → review+
Convert assert to error return per review comment. Carrying forward r=kinetik.
Attachment #8682104 - Attachment is obsolete: true
Attachment #8682169 - Flags: review+
This patch will be functional once bug 1219530 lands. Until then you need to #define MOZ_RUST_MP4PARSE in media/libstagefright/binding/MP4Metadata.cpp.
Flags: needinfo?(ajones)
Blocks: 1220882
Blocks: 1220885
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
You need to log in before you can comment on or make changes to this bug.