Support audio profile in mp4 rust parser

RESOLVED FIXED in Firefox 53

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: alfredo, Assigned: alfredo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

a year ago
Assignee: nobody → ayang
Blocks: 1161350
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8818487 [details]
Bug 1323390 - Support audio profile in mp4 rust parser.

https://reviewboard.mozilla.org/r/98550/#review98932
Attachment #8818487 - Flags: review?(kinetik) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

a year ago
After trying almost for a day, I still failed to add 'bitreader' into gecko.
I have tried following ways.

1. I followed the steps in https://gecko.readthedocs.io/en/latest/build/buildsystem/rust.html#crate-dependencies. But it failed at 'mach vendor rust'.

2. I added 'bitreader' into third_party/rust manually, but it fails when checking '.cargo-checksum.json' in bitreader. After looking for a while, I can't find how to generate this file.

Any suggestion?
Flags: needinfo?(giles)
Ok. I tried declaring bitreader as a dep of mp4parse:

> index affcef72b022..c67d4c0965a0 100644
> --- a/media/libstagefright/binding/mp4parse/Cargo.toml
> +++ b/media/libstagefright/binding/mp4parse/Cargo.toml
> @@ -20,6 +20,7 @@ exclude = [
>  
>  [dependencies]
>  byteorder = "0.5.0"
> +bitreader = "0.1.0"
>  
>  [dev-dependencies]
>  test-assembler = "0.1.2"

Then I ran `./mach vendor rust`. This initially failed trying to install `cargo-vendor` because of missing openssl on MacOS. I worked around this by running `cargo install cargo-vendor` with appropriate flags, and tried to automate this in bug 1324869.

Running `./mach vendor rust` again downloaded and unpacked a copy of the bitreader source in third_party/rust. It also updated many other dependencies, which should probably be a separate commit. After that `./mach build` completed, showing bitreader was compiled, and adding some dummy calls to mp4parse/src/lib.rs showed no link errors.

What error message did you get from `mach vendor rust`?
Flags: needinfo?(giles)
I tried to reduce the 'mach vendor' diff noise by updating some of the other thirty-party crates in bug 1324920.
(Assignee)

Comment 9

a year ago
(In reply to Ralph Giles (:rillian) needinfo me from comment #7)
> Ok. I tried declaring bitreader as a dep of mp4parse:
> 
> > index affcef72b022..c67d4c0965a0 100644
> > --- a/media/libstagefright/binding/mp4parse/Cargo.toml
> > +++ b/media/libstagefright/binding/mp4parse/Cargo.toml
> > @@ -20,6 +20,7 @@ exclude = [
> >  
> >  [dependencies]
> >  byteorder = "0.5.0"
> > +bitreader = "0.1.0"
> >  
> >  [dev-dependencies]
> >  test-assembler = "0.1.2"
> 
> Then I ran `./mach vendor rust`. This initially failed trying to install
> `cargo-vendor` because of missing openssl on MacOS. I worked around this by
> running `cargo install cargo-vendor` with appropriate flags, and tried to
> automate this in bug 1324869.
> 
> Running `./mach vendor rust` again downloaded and unpacked a copy of the
> bitreader source in third_party/rust. It also updated many other
> dependencies, which should probably be a separate commit. After that `./mach
> build` completed, showing bitreader was compiled, and adding some dummy
> calls to mp4parse/src/lib.rs showed no link errors.
> 
> What error message did you get from `mach vendor rust`?


(38fe301|MERGE)alfredo@Power-Moz:mozilla-unified$ ./mach vendor rust --ignore-modified
0:00.27 Installing cargo-vendor
0:00.27 //Users/alfredo/.cargo/bin/cargo install cargo-vendor
Error running mach:

    ['vendor', 'rust', '--ignore-modified']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

Exception: Process executed with non-0 exit code 101: ['//Users/alfredo/.cargo/bin/cargo', u'install', u'cargo-vendor']

  File "/Users/alfredo/mozilla/mozilla-unified/python/mozbuild/mozbuild/mach_commands.py", line 1610, in vendor_rust
    vendor_command.vendor(**kwargs)
  File "/Users/alfredo/mozilla/mozilla-unified/python/mozbuild/mozbuild/vendor_rust.py", line 71, in vendor
    self.run_process(args=[cargo, 'install', 'cargo-vendor'])
  File "/Users/alfredo/mozilla/mozilla-unified/python/mach/mach/mixin/process.py", line 147, in run_process
    raise Exception('Process executed with non-0 exit code %d: %s' % (status, args))
 (38fe301|MERGE)alfredo@Power-Moz:mozilla-unified$
Ok. I didn't try `--ignore-modified` but that shouldn't make a difference. It looks like the install failed. Unfortunately, mach doesn't forward the underlying error message in this case.

You can try running `cargo install cargo-vendor` yourself and see if that gives a more clear error.

You can also try rebasing and running `./mach vendor rust` again now that bug 1324462 and (if you're on mac) bug 1324869 have landed.
(Assignee)

Comment 11

a year ago
(In reply to Ralph Giles (:rillian) needinfo me from comment #10)
> Ok. I didn't try `--ignore-modified` but that shouldn't make a difference.
> It looks like the install failed. Unfortunately, mach doesn't forward the
> underlying error message in this case.
> 
> You can try running `cargo install cargo-vendor` yourself and see if that
> gives a more clear error.
> 
> You can also try rebasing and running `./mach vendor rust` again now that
> bug 1324462 and (if you're on mac) bug 1324869 have landed.

`./mach vendor rust` works now after rebasing! Thank you.
Comment hidden (mozreview-request)

Comment 13

a year ago
Pushed by ayang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a4e7ea691710
Support audio profile in mp4 rust parser. r=kinetik

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a4e7ea691710
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.