Update cubeb from upstream to 087dc94

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kamidphish, Assigned: kamidphish)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

No description provided.
Assignee: nobody → dglastonbury
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Pick commits
4efb2e6 [Dan Glastonbury] Rework device collection (#309)
c9641ca [Matthew Gregan] Add backend specification support to common.h (djg/dev, djg/HEAD)
0c74372 [Matthew Gregan] Detabify.

Comment 6

2 years ago
mozreview-review
Comment on attachment 8871599 [details]
Bug 1367646 - Update cubeb from upstream to 087dc94.

https://reviewboard.mozilla.org/r/143072/#review146784
Attachment #8871599 - Flags: review?(kinetik) → review+

Comment 7

2 years ago
mozreview-review
Comment on attachment 8871600 [details]
Bug 1367646 - Let cubeb_device_collection_destroy handle invalid collections.

https://reviewboard.mozilla.org/r/143074/#review146786
Attachment #8871600 - Flags: review?(kinetik) → review+

Comment 8

2 years ago
mozreview-review
Comment on attachment 8871601 [details]
Bug 1367646 - Add device collection patch to libcubeb/update.h

https://reviewboard.mozilla.org/r/143076/#review146788

Would be better to just upstream this and update the initial import IMO
Attachment #8871601 - Flags: review?(kinetik) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8871602 [details]
Bug 1367646 - Update WebRTC with cubeb_device_collection changes. r+jesup

https://reviewboard.mozilla.org/r/143078/#review146790

LGTM but it's not my code to review
Attachment #8871602 - Flags: review?(kinetik) → review?(rjesup)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8871602 [details]
Bug 1367646 - Update WebRTC with cubeb_device_collection changes. r+jesup

https://reviewboard.mozilla.org/r/143078/#review147354

::: dom/media/webrtc/MediaEngineWebRTC.h:270
(Diff revision 1)
> -    MOZ_ASSERT(!mDevices);
> +    MOZ_ASSERT(!mDevices.count);
>  #else
> -    MOZ_ASSERT(mDevices);
> +    MOZ_ASSERT(mDevices.count);

Very slight preference for using comparisons with numbers for integers -- MOZ_ASSERT(mDevices.count == 0), MOZ_ASSERT(mDevices.count > 0), but only very slight.
Attachment #8871602 - Flags: review?(rjesup) → review+
(In reply to Randell Jesup [:jesup] from comment #10)
> Comment on attachment 8871602 [details]
> Bug 1367646 - Update WebRTC with cubeb_device_collection changes.
> 
> https://reviewboard.mozilla.org/r/143078/#review147354
> 
> ::: dom/media/webrtc/MediaEngineWebRTC.h:270
> (Diff revision 1)
> > -    MOZ_ASSERT(!mDevices);
> > +    MOZ_ASSERT(!mDevices.count);
> >  #else
> > -    MOZ_ASSERT(mDevices);
> > +    MOZ_ASSERT(mDevices.count);
> 
> Very slight preference for using comparisons with numbers for integers --
> MOZ_ASSERT(mDevices.count == 0), MOZ_ASSERT(mDevices.count > 0), but only
> very slight.

Not a problem.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8871600 - Attachment is obsolete: true
Attachment #8871601 - Attachment is obsolete: true
Additional commits:
087dc94 cubeb: Test cubeb_device_collection_destroy behaviour.
040cf4a cubeb: Change cubeb_device_collection_destroy to handle invalid data.
62871b2 wasapi: use frames instead of samples for capture-only also.
e7f7ebb wasapi: use number of frames instead of samples
f401f7e cubeb_get_preferred_channel_layout should be verified in test_sanity instead of test_latency
66f0e16 Correct the check for mixed results
444c3cd Add assertions to check we don't write out of bound of buffer
290e635 Add osx-specific setting for downmixing test
Summary: Update cubeb from upstream to 4efb2e6 → Update cubeb from upstream to 087dc94

Comment 15

2 years ago
mozreview-review
Comment on attachment 8871602 [details]
Bug 1367646 - Update WebRTC with cubeb_device_collection changes. r+jesup

https://reviewboard.mozilla.org/r/143078/#review147402
Attachment #8871602 - Flags: review?(kinetik) → review+
Bug 1368225 is trying to land libcubeb updates for the range included in comment 14, so if you land this patch that bug can be obsoleted.
Duplicate of this bug: 1368225

Comment 18

2 years ago
Pushed by dglastonbury@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e21dcef641e4
Update cubeb from upstream to 087dc94. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/75eef8b3b1a5
Update WebRTC with cubeb_device_collection changes. r+jesup r=jesup,kinetik

Comment 19

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e21dcef641e4
https://hg.mozilla.org/mozilla-central/rev/75eef8b3b1a5
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
as a note, this increased (i.e. regressed) the compile warnings for linux64 debug static analysis:
== Change summary for alert #6935 (as of May 30 2017 00:38 UTC) ==

Regressions:

  2%  compiler warnings summary linux64 debug static-analysis     364.00 -> 369.67

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6935

there are no rules to fix this, so use this data for information purposes only, maybe to clean up a few warnings :)
(In reply to Joel Maher ( :jmaher) from comment #20)
> as a note, this increased (i.e. regressed) the compile warnings for linux64
> debug static analysis:
> == Change summary for alert #6935 (as of May 30 2017 00:38 UTC) ==
> 
> Regressions:
> 
>   2%  compiler warnings summary linux64 debug static-analysis     364.00 ->
> 369.67
> 
> For up to date results, see:
> https://treeherder.mozilla.org/perf.html#/alerts?id=6935
> 
> there are no rules to fix this, so use this data for information purposes
> only, maybe to clean up a few warnings :)

Hi :jmaher, thanks for the heads up. Is there a way to see the new warnings from the treeherder alert page?

There are some warning fixes coming in upstream cubeb soon. https://github.com/kinetiknz/cubeb/pull/316
I am not aware of ways to see what warnings, :gps had enabled these data points to be sent to perfherder, possibly he has a tool or a trick to see a diff of the warnings
Flags: needinfo?(gps)
We don't yet have a tool to diff the warnings.

I think most warnings from 3rd party code should be suppressed via compiler flag adjustment unless we actually want to help fix them upstream or patch them locally. Otherwise, they are effectively noise.
Flags: needinfo?(gps)
You need to log in before you can comment on or make changes to this bug.