Closed Bug 1331869 Opened 3 years ago Closed 3 years ago

Update cubeb from upstream to d96e35f02d

Categories

(Core :: Audio/Video: cubeb, defect, P1)

53 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: achronop, Assigned: padenot)

References

Details

Attachments

(4 files, 1 obsolete file)

Update cubeb from upstream to 00ee120370
Assignee: nobody → achronop
Rank: 15
Priority: -- → P1
Blocks: 1326176
Blocks: 1317234
Blocks: 1328284
We want this on 52 too.
Yeah we'll cherry pick the relevant stuff.
Summary: Update cubeb from upstream to 00ee120370 → Update cubeb from upstream to d96e35f02d
Comment on attachment 8827895 [details]
Bug 1331869 - Update moz.build to build cubeb_mixer.cpp.

https://reviewboard.mozilla.org/r/105502/#review106750

I cancel this one to create a new.
Attachment #8827895 - Flags: review?(achronop)
Comment on attachment 8827855 [details]
Bug 1331869 - Update cubeb from upstream to d96e35f02d.

https://reviewboard.mozilla.org/r/105476/#review106752

I cancel this one also.
Attachment #8827855 - Flags: review?(padenot)
Attachment #8827895 - Attachment is obsolete: true
Comment on attachment 8827855 [details]
Bug 1331869 - Update cubeb from upstream to d96e35f02d.

https://reviewboard.mozilla.org/r/105476/#review106884
Attachment #8827855 - Flags: review?(kinetik) → review+
Comment on attachment 8828402 [details]
Bug 1331869 - Initialize new field of cubeb_stream_params.

https://reviewboard.mozilla.org/r/105832/#review106886
Attachment #8828402 - Flags: review?(kinetik) → review+
Comment on attachment 8828402 [details]
Bug 1331869 - Initialize new field of cubeb_stream_params.

https://reviewboard.mozilla.org/r/105832/#review107044

::: dom/media/GraphDriver.cpp:666
(Diff revision 1)
>    }
>  
>  
>    input = output;
>    input.channels = mInputChannels; // change to support optional stereo capture
> +  input.layout = (mInputChannels == 1) ? CUBEB_LAYOUT_MONO : CUBEB_LAYOUT_STEREO;

Just hardcode this to mono, some other code is not ready for stereo input, iirc.
Attachment #8828402 - Flags: review?(padenot) → review+
Comment on attachment 8827855 [details]
Bug 1331869 - Update cubeb from upstream to d96e35f02d.

https://reviewboard.mozilla.org/r/105476/#review107048
Attachment #8827855 - Flags: review?(padenot) → review+
Pushed by achronop@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/056197b55d49
Update cubeb from upstream to d96e35f02d. r=kinetik,padenot
https://hg.mozilla.org/integration/autoland/rev/2d93fb71eb88
Initialize new field of cubeb_stream_params. r=kinetik,padenot
https://hg.mozilla.org/mozilla-central/rev/056197b55d49
https://hg.mozilla.org/mozilla-central/rev/2d93fb71eb88
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Alex, according to comment #3, we want this in 52. could you fill the uplift request? Thanks
Flags: needinfo?(achronop)
I've rebased the mac-only part of this, and I've pushed to try, it's currently retriggering mda a bunch to make sure:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8eec8fd545d3fc8429812b9cf7910aa50a591908
Clearing NI based on previous comment
Flags: needinfo?(achronop)
Assignee: achronop → padenot
This patch is quite big because:
- It's moving two very big function around ;
- It's renaming some stuff ;
- It includes a copy of the patch in the patch (see update.sh changes, when uplifting cubeb revision, we prefer to do it this way so that we know where all the modifications come from).

The actual changes are not too big. I feel confident about uplifting.
Comment on attachment 8830393 [details] [diff] [review]
Uplift the OSX portion of bug 1331869 to beta

Looks good, thanks
Attachment #8830393 - Flags: review?(achronop) → review+
Comment on attachment 8830393 [details] [diff] [review]
Uplift the OSX portion of bug 1331869 to beta

Approval Request Comment
[Feature/Bug causing the regression]: Rather old deadlock on OSX (multiple years old at least)
[User impact if declined]: Frequent oranges on automation, deadlocks in the wild, when playing back audio on OSX
[Is this code covered by automated tests?]: No, but sherrifs observed that the oranges are gone. We haven't been able to repro the deadlock reliably to write a unit test.
[Has the fix been verified in Nightly?]: Yes, it has proven to be effective
[Needs manual test from QE? If yes, steps to reproduce]:  No
[List of other uplifts needed for the feature/fix]: None, this is a rollup patch that includes everything
[Is the change risky?]: I don't think it is. Also this can be backed out cleanly.
[Why is the change risky/not risky?]: No other problems have been reported in aurora or nightly with this patch, and it has fixed a number of oranges. 
[String changes made/needed]: None
Attachment #8830393 - Flags: approval-mozilla-beta?
Comment on attachment 8830393 [details] [diff] [review]
Uplift the OSX portion of bug 1331869 to beta

cubeb update to fix osx deadlock, beta52+
Attachment #8830393 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8831621 [details] [diff] [review]
Uplift a double-free fix (cubeb revision f82f15)

This is fixed on central and aurora and fixes a low volume crash.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1302231
[User impact if declined]: low volume crash
[Is this code covered by automated tests?]: no, we're not even able to reproduce
[Has the fix been verified in Nightly?]: Yes, and also on aurora (approximately a week of baking each)
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Can be backed out cleanly. Crash stats clearly shows this is effective.
[Why is the change risky/not risky?]: Not risky, this simply adds a check fore deleting some data
[String changes made/needed]: None
Attachment #8831621 - Flags: approval-mozilla-beta?
Comment on attachment 8831621 [details] [diff] [review]
Uplift a double-free fix (cubeb revision f82f15)

one more cubeb fix for beta52
Attachment #8831621 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1439407
You need to log in before you can comment on or make changes to this bug.