FWIW, this one has existed for several years. It shows up on Chrome OS as well.

This is a third_party library not Google-owned, I'm not sure we want to make contributions to https://github.com/cisco/openh264 or maintain our own fork for the sake of a couple of static initializers?

Reassigned to OpenH264 person.

To be clear: Getting rid of this static initializer is important. We have a strict policy of not allowing regressions in number of static initializers, we've delayed feature launches because they brought in libs using static initializers, and we've been at 0 static initializers on mac for like 6 years. So please don't call this "a couple of static initializers" and please fix it as soon as possible.

In general, contributing patches upstream has a lower long-term cost, so if that's possible that's what I'd recommend. If it isn't, we need to carry a downstream patch.

I've created a pull request for OpenH264: https://github.com/cisco/openh264/pull/3029

In the past, they've accepted my pull request within a day so I hope that they can accept this one.

Awesome, thanks!

https://github.com/cisco/openh264/pull/3029/files -- 

CWelsLock& GetInitLock() {
  static CWelsLock initLock;
  return initLock;
}

This fixes the static ctor, but still creates an exit-time dtor. Maybe `static CWelsLock *initLock = new CWelsLock; return *initLock;` is better?

> This fixes the static ctor, but still creates an exit-time dtor. Maybe `static CWelsLock *initLock = new CWelsLock; return *initLock;` is better?

Done!  forgot about that

Thanks for taking this on, I appreciate it.

Since this isn't merged yet, can we carry this as downstream diff?

Over to sprang@ for c#8

spang: Ping. This causes SI expectations to be different for official and regular builds, which makes many things very confusing. Let's get this fixed, please.

*sprang: ...

I have no idea if we can add a downstream diff to this. We're pulling from the github repo right? Do we even have a standard way of adding patches on top of that?

How long does it typically take to have a change such as this merged? I think just waiting for that and rolling the openh264 version is easier.

I'm not an owner of this component and not a even a chrome committer, so if you want this resolved very quickly this should probably be reassigned.

Who owns OpenH264 in chrome? what do we do when we need to quickly patch security issues?

hbos@ you'r at least in the OWNERS list. Do you know how patching would work?

I don't know how it works, I haven't touched this in several years and we've never maintained our own patches. Maybe ask chromium-third-party@google.com?

chromium-third-party is for legal reviews as far as I know (?)

+mflodman, the other entry in third_party/openh264/OWNERS. Do you know anything about how we handle security patches to openh264?

Might want to hold off on patching for now since it looks like this might be getting merged upstream soon: https://github.com/cisco/openh264/pull/3029#issuecomment-434301344

Re #17
No, hbos has been the main person dealing with this from our side. AFAIK we haven't handled any security patches until know.

I was finally able to reach someone who could merge the pull request. What's the next step? Rolling openh264?

I'll send out a roll CL soon

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f688af0a875cd329bc64fd38099ec429eb6caf80

commit f688af0a875cd329bc64fd38099ec429eb6caf80
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Thu Nov 29 23:45:22 2018

Roll openh264 to 6f26bce

Changelog
https://chromium.googlesource.com/external/github.com/cisco/openh264/+log/3b51f16a4a41df729f8d647f03e48c5f272911ff..6f26bce0b1c4e8ce0e13332f7c0083788def5fdf

BUG= 893594 
TBR=mflodman,hbos

Change-Id: I15409f2d1671f23fb785d63f1d3b2f1b0d4b5316
Reviewed-on: https://chromium-review.googlesource.com/c/1355253
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612440}
[modify] https://crrev.com/f688af0a875cd329bc64fd38099ec429eb6caf80/DEPS
[modify] https://crrev.com/f688af0a875cd329bc64fd38099ec429eb6caf80/third_party/openh264/README.chromium

Cool. Now we only need to lower static initializer expectations by 1 and this is all done, right?

I think so. https://chromium-review.googlesource.com/c/chromium/src/+/1356868

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28cb5d154f7dc3d4027ded9b7c09c6c6b6c998d7

commit 28cb5d154f7dc3d4027ded9b7c09c6c6b6c998d7
Author: Robert Sesek <rsesek@chromium.org>
Date: Fri Nov 30 18:48:00 2018

Decrease the expected number of static initializers by 1.

This brings EXPECTED_MAC_SI_COUNT back to 0, and reduces the
number in Linux by 1. The SI in WelsThreadPool.cpp was removed.

Bug:  893594 
Change-Id: I6fc03be4b104a5f8afc4c2be1cebce36d322d116
Reviewed-on: https://chromium-review.googlesource.com/c/1356868
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612709}
[modify] https://crrev.com/28cb5d154f7dc3d4027ded9b7c09c6c6b6c998d7/infra/scripts/legacy/scripts/slave/chromium/sizes.py

Thanks for the team effort on this!

https://logs.chromium.org/logs/chromium/bb/chromium.chrome/Google_Chrome_Mac/39233/+/recipes/steps/sizes/0/stdout