1646904 - [meta] Decide which local changes to libwebrtc need to be addressed before the next update

Status: NEW

(Core :: WebRTC, task)

Description

[Nico Grunbaum [:ng, @chew:mozilla.org]]

In order to land the next libwebrtc update we need to decide which of these bugs can be removed as duplicates of features upstream (i.e. they were cherry picked), which can be reimplmented using upstream functionality, which can be implemented via workarounds, and which must persist.

Comment 1

[Dan Shick]

1496359 has only been closed in favor of 1430775 which didn't make this list. If it isn't possible to modify the build configuration to support the appropriate flag, maybe some workaround can be considered? Currently, distros supporting Wayland are forced to patch this downstream.

Comment 2

[Dan Minor [:dminor]]

Starting to work through this list. Bug 1376873 was the update to branch 64 bug, we'll need to go through those patches individually and might want to breakup the "rollup" patch into chunks for separate review.

Cherry picks / partial updates from upstream:
Bug 813063, Bug 1411681, Bug 1480088, Bug 1496359

No longer relevant:
Bug 1368816 - re-enables a gtest, but we stopped running webrtc gtests.
Bug 1439997 - made modifications to os x video capture code, but upstream has removed that, so we're maintaining this on our own.
Bug 1486038 - added workarounds for msvc builds, removed in Bug 1515699.
Bug 1497552 - removed local modifications.
Bug 1497559 - removed application capture.
Bug 1497573 - removed local modifications.
Bug 1497577 - removed local modifications.
Bug 1497602 - removed local modifications.
Bug 1497606 - removed local modifications.
Bug 1497610 - removed local modifications.
Bug 1497619 - removed local modifications.
Bug 1497650 - removed local modifications.
Bug 1497974 - removed local modifications.
Bug 1497992 - removed local modifications.
Bug 1498205 - moved some of our local modifications to dom/media/systemservices.
Bug 1498253 - removed local modifications.
Bug 1503363 - modified moz.build file, so no conflicts from upstream.
Bug 1506644 - added gn generated json files, will need to regenerate as part of doing update anyway.
Bug 1509994 - moved some of our local modifications to dom/media/systemservices.
Bug 1512459 - removed dead code for sndio audio device.
Bug 1512504 - updated moz.build files, so no conflicts from upstream.
Bug 1515461 - made modifications to os x video capture code, but upstream has removed that, so we're maintaining this on our own.
Bug 1518744 - made modifications to os x video capture code, but upstream has removed that, so we're maintaining this on our own.
Bug 1519535 - made modifications to os x video capture code, but upstream has removed that, so we're maintaining this on our own.

Comment 3

[Dan Minor [:dminor]]

Cherry picks / partial updates from upstream: Bug 1545247, Bug 1552755, Bug 1554949, Bug 1559011, Bug 1560145

Landed upstream: Bug 1523412, Bug 1580156,

No longer relevant:
Bug 1522238 - removed local modifications.
Bug 1523162 - removed local modifications.
Bug 1523969 - doesn't appear to touch libwebrtc code.
Bug 1525341 - Can be dropped per https://bugzilla.mozilla.org/show_bug.cgi?id=1525341#c8
Bug 1525393 - These were local modifications required to get things building with the libvpx update. We want to drop these changes with the next update.
Bug 1528074 - Updates a moz.build file.
Bug 1532465 - Applies to code we've subsequently removed.
Bug 1564902 - Fix for same problem already present upstream.
Bug 1565800 - Fix to android capture code that has been moved to dom/media/systemservices.
Bug 1578073 - Pulls in android camera code from upstream and moves our own code to dom/media/systemservices.
Bug 1579834 - Added a build configuration for mips.
Bug 1583967 - Already fixed upstream.

Comment 4

[Dan Minor [:dminor]]

Cherry picks: Bug 1611486, Bug 1619484, Bug 1632489.

Landed upstream: Bug 1603296, Bug 1637319.

No longer relevant:
Bug 1587159 - Upstream code has been rewritten.
Bug 1588346 - Removed unused code.
Bug 1590984 - This is a fix to our existing local modifications, so doesn't need to be reviewed separately.
Bug 1593161 - This is a fix to OS X video capture code which has been removed upstream, so we maintain it now anyway.
Bug 1625288 - Reverts a broken fix for OS X video capture code.
Bug 1625694 - Removes some local modifications and the rest can be dropped, problem fixed in a different way upstream.
Bug 1629984 - Fix to OS X camera capture code, which we own now anyway.
Bug 1634675 - Removed webrtc-gtest.
Bug 1645127 - Similar fix exists upstream, this can be dropped.
Bug 1646150 - Removed unused code.

Comment 5

[Dan Minor [:dminor]]

Bug 864513 tracks things to upstream or remove noted during the last webrtc.org update. There's still a lot of open bugs there :/.

I've flagged a bunch of things as not relevant because they removed local modifications. We'll still have to be careful we don't accidentally re-introduce those local modifications when we do the update. We just don't need to worry about upstreaming or removing them.

Comment 6

[June Wilde (she/her) [:jewilde]]

I was attempting to get 1395259 upstreamed in this chromium bug but sadly it hasn't gone anywhere since earlier this year. The patch attached there is a modified version of what we have locally that should work on webrtc's latest

Comment 7

[Dan Minor [:dminor]]

The modifications from bug 1515205 were removed in Bug 1522238.

Comment 8

[Dan Minor [:dminor]]

The changes in Bug 1524335 were to the rtc::Optional type, which has been removed upstream in favour of abseil.

Comment 9

[Dan Minor [:dminor]]

Bug 1621500 was reverted in Bug 1625288, and only affects the OS X video capture code, which we maintain now outside of libwebrtc.

Comment 10

[Dan Minor [:dminor]]

Bug 1598543 was a security bug that was reported upstream and fixed independently there. We can probably drop our local modifications, but should verify with the test cases that Jib put together.

Comment 11

[Dan Minor [:dminor]]

(In reply to Dan Minor [:dminor] from comment #10)

Bug 1598543 was a security bug that was reported upstream and fixed independently there. We can probably drop our local modifications, but should verify with the test cases that Jib put together.

I poked into this a little bit and it looks like this was never fully fixed upstream. We should try to upstream the actual fix while dropping some of the changes that were made to cover things up.

Comment 12

[Dan Minor [:dminor]]

Bug 1587164 just landed upstream.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The meta keyword is there, the bug doesn't depend on other bugs and there is no activity for 12 months.
:mjf, maybe it's time to close this bug?

Comment 14

[Michael Froman [:mjf]]

Still working on the uplift, so we'll keep this open for now.

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The meta keyword is there, the bug doesn't depend on other bugs and there is no activity for 12 months.
:mjf, maybe it's time to close this bug?