1242807 - Fix mp4parse-rust's error reporting via telemetry

Status: RESOLVED FIXED

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

Description

[Matthew Gregan [:kinetik]]

Bug 1238420 added telemetry for recording mp4parse-rust's parsing errors.

Unfortunately the results reported were garbage due to two small bugs:

1. try_rust wasn't updated to return an int32_t error code (instead returned bool)
2. mp4parse_read's error remapping from the JoinHandle forgot to unwrap the Ok() result

Trivial fix coming up.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: v0

We can either hotfix capi.rs or wait until the next import of mp4parse-rust.  I don't mind either way, but it'd be nice to get good telemetry results sooner rather than later.

Comment 2

[Ralph Giles (:rillian)]

Comment on attachment 8711963 [details] [diff] [review]
v0

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

ship it!

::: media/libstagefright/binding/capi.rs
@@ +124,4 @@
>      // result in an Ok(..) otherwise, meaning we could see
>      // Ok(Err(Error::..)) here. So map thread failures back
>      // to an mp4parse::Error before converting to a C return value.
> +    match task.join().unwrap_or(Err(Error::AssertCaught)) {

Ouch. Even with the comment, this Result<Result<_>, ...> stuff is confusing!

Comment 3

[Matthew Gregan [:kinetik]]

Thanks for the quick review!

The capi.rs change is upstream(ish) at https://github.com/kinetiknz/mp4parse-rust/commit/b6d562127c879b0dbaf1bfccf1f8004568a4f953

Comment 4

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3c7b87718d4b3b3ceae8c9947a9f3eecac8a4eb
Bug 1242807 - Fix mp4parse-rust's error reporting via telemetry.  r=giles

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e3c7b87718d4