1219047 - Call rust mp4parser from MP4Metadata

Status: RESOLVED FIXED

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

Description

[Ralph Giles (:rillian)]

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.

Comment 1

[Ralph Giles (:rillian)]

Attachment: Call rust mp4 parser v1

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

Comment 2

[Matthew Gregan [:kinetik]]

Pushed a version with MOZ_RUST conditionals (needs bug 1219530) to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fb0897f1f98

I don't see the shutdown crash on Linux x64 debug with rustc 1.3.0... it'd be good to track that down.

Comment 3

[Ralph Giles (:rillian)]

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.

Comment 4

[Ralph Giles (:rillian)]

Attachment: Call rust mp4 parser v2

Let's get something landed.

Comment 5

[Matthew Gregan [: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.

Comment 6

[Ralph Giles (:rillian)]

Attachment: Call rust mp4 parser v3

Convert assert to error return per review comment. Carrying forward r=kinetik.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a48d8551417

Comment 8

[Ralph Giles (:rillian)]

This patch will be functional once bug 1219530 lands. Until then you need to #define MOZ_RUST_MP4PARSE in media/libstagefright/binding/MP4Metadata.cpp.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4a48d8551417

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/4a48d8551417

Comment 11

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion