Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2947273002:
Fix the binary size regression on Chromium Windows. (Closed)
DescriptionFix the binary size regression on Chromium Windows.
There is a dependency chain from Chromium windows main_dll to Opus
which should never exist. We used to rely on rtc_static_library
to break this chain. So this CL replaced some rtc_source_set
with rtc_static_library.
libvpx fix (https://chromium-review.googlesource.com/c/544107/) for
ios-simulator linking issue is landed and this CL can be sumbitted once the new
Chromium is rolled into WebRTC.
BUG=chromium:734631
Review-Url: https://codereview.webrtc.org/2947273002
Cr-Commit-Position: refs/heads/master@{#18709}
Committed: https://chromium.googlesource.com/external/webrtc/+/ab97e18fa97a73d8ad0ac51c11d7111dd397984d
Patch Set 1 #Patch Set 2 : Change to rtc_source_set to rtc_static_library to break the dependency from chromium.dll to opus. #
Messages
Total messages: 31 (23 generated)
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by zhihuang@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== Fix the binary size regression on Chromium Windows. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. BUG=chromium:734631 ==========
zhihuang@webrtc.org changed reviewers: + kjellander@webrtc.org
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. Should wait until this CL landed first to avoid the linking issue on ios-simulator: https://chromium-review.googlesource.com/c/544107/ BUG=chromium:734631 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...)
zhihuang@webrtc.org changed reviewers: + deadbeef@webrtc.org
PTAL. This should fix the issue.
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. Should wait until this CL landed first to avoid the linking issue on ios-simulator: https://chromium-review.googlesource.com/c/544107/ BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. Should wait until this CL landed first to avoid the linking issue on ios-simulator: Libvpx fix (https://chromium-review.googlesource.com/c/544107/)for ios-simulator issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. Should wait until this CL landed first to avoid the linking issue on ios-simulator: Libvpx fix (https://chromium-review.googlesource.com/c/544107/)for ios-simulator issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. Libvpx fix (https://chromium-review.googlesource.com/c/544107/)for ios-simulator issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. Libvpx fix (https://chromium-review.googlesource.com/c/544107/)for ios-simulator issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. Using rtc_static_library can break this chain so that the binary size won't grow. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain through Chromium windows to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed, this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed and this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
I don't understand what's going on here; does chromium on Windows not actually need libopus? Why not? And why does static_library make a difference here?
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed and this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows main_dll to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed and this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ==========
On 2017/06/22 05:26:24, Taylor Brandstetter wrote: > I don't understand what's going on here; does chromium on Windows not actually > need libopus? Why not? And why does static_library make a difference here? The tricky part is that the opus shouldn't be linked in Chromium main_dll which is a special case on Windows and it should be linked to some other DLLs I imagine. The optimization of rtc_static_library would break this chain. But the long term solution should be remove this dependency chain. The libvpx changes is used to solve the linking issue on ios_simulator.
The CQ bit was checked by sakal@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The libvpx change was rolled in 11 minutes ago in https://chromium.googlesource.com/external/webrtc/+/86e7ef83b38ef59c6a799c59f... so this is good to go. lgtm
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/06/22 07:36:36, sakal wrote: > lgtm Landing this now. Taylor: let us know if you have outstanding questions/concerns and worst case revert.
The CQ bit was checked by kjellander@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498120034824800,
"parent_rev": "86e7ef83b38ef59c6a799c59f4ade37e3509c5af", "commit_rev":
"ab97e18fa97a73d8ad0ac51c11d7111dd397984d"}
Message was sent while issue was closed.
Description was changed from ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows main_dll to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed and this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 ========== to ========== Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows main_dll to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed and this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG=chromium:734631 Review-Url: https://codereview.webrtc.org/2947273002 Cr-Commit-Position: refs/heads/master@{#18709} Committed: https://chromium.googlesource.com/external/webrtc/+/ab97e18fa97a73d8ad0ac51c1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/external/webrtc/+/ab97e18fa97a73d8ad0ac51c1... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
