Closed Bug 1367646 Opened 6 years ago Closed 6 years ago

Update cubeb from upstream to 087dc94

Categories

(Core :: Audio/Video: cubeb, enhancement)

Product:
Core
Component:
Audio/Video: cubeb
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: u480271, Assigned: u480271)

References

Details

Attachments

(2 files, 2 obsolete files)

Bug 1367646 - Update cubeb from upstream to 087dc94.
59 bytes, text/x-review-board-request
kinetik
: review+
Details
Bug 1367646 - Update WebRTC with cubeb_device_collection changes. r+jesup
59 bytes, text/x-review-board-request
kinetik
: review+
Details 
      No description provided.
Assignee: nobody → dglastonbury
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 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 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 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 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 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.
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 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.
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
Blocks: 1360060
https://hg.mozilla.org/mozilla-central/rev/e21dcef641e4
https://hg.mozilla.org/mozilla-central/rev/75eef8b3b1a5
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox55: --- → fixed
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.