H264: Thread-safe InitializeFFmpeg. Flag to control if InitializeFFmpeg should be called.
New flag: rtc_initialize_ffmpeg, default value = !build_with_chromium.
In WebRTC standalone we initialize FFmpeg by default, in Chromium we don't by default.
Chromium is an external project that also use FFmpeg. If both projects do FFmpeg initialization code things will break. The flag makes it possible for other external projects than chromium to decide whether or not WebRTC should initialize FFmpeg.
BUG=chromium:500605, chromium:468365, webrtc:5427
Review URL: https://codereview.webrtc.org/1639273002
Cr-Commit-Position: refs/heads/master@{#11456}
diff --git a/webrtc/build/common.gypi b/webrtc/build/common.gypi
index a77420c..d5b79f9 100644
--- a/webrtc/build/common.gypi
+++ b/webrtc/build/common.gypi
@@ -142,11 +142,23 @@
# support, |ffmpeg_branding| has to separately be set to a value that
# includes H.264, for example "Chrome". If FFmpeg is built without H.264,
# compilation succeeds but |H264DecoderImpl| fails to initialize.
+ # See also: |rtc_initialize_ffmpeg|.
# CHECK THE OPENH264, FFMPEG AND H.264 LICENSES/PATENTS BEFORE BUILDING.
# http://www.openh264.org, https://www.ffmpeg.org/
'rtc_use_h264%': 0,
'conditions': [
+ # FFmpeg must be initialized for |H264DecoderImpl| to work. This can be
+ # done by WebRTC during |H264DecoderImpl::InitDecode| or externally.
+ # FFmpeg must only be initialized once. Projects that initialize FFmpeg
+ # externally, such as Chromium, must turn this flag off so that WebRTC
+ # does not also initialize.
+ ['build_with_chromium==0', {
+ 'rtc_initialize_ffmpeg%': 1,
+ }, {
+ 'rtc_initialize_ffmpeg%': 0,
+ }],
+
['build_with_chromium==1', {
# Exclude pulse audio on Chromium since its prerequisites don't require
# pulse audio.
diff --git a/webrtc/build/webrtc.gni b/webrtc/build/webrtc.gni
index f784ab1..886b899 100644
--- a/webrtc/build/webrtc.gni
+++ b/webrtc/build/webrtc.gni
@@ -96,9 +96,17 @@
# support, |ffmpeg_branding| has to separately be set to a value that
# includes H.264, for example "Chrome". If FFmpeg is built without H.264,
# compilation succeeds but |H264DecoderImpl| fails to initialize.
+ # See also: |rtc_initialize_ffmpeg|.
# CHECK THE OPENH264, FFMPEG AND H.264 LICENSES/PATENTS BEFORE BUILDING.
# http://www.openh264.org, https://www.ffmpeg.org/
rtc_use_h264 = false
+
+ # FFmpeg must be initialized for |H264DecoderImpl| to work. This can be done
+ # by WebRTC during |H264DecoderImpl::InitDecode| or externally. FFmpeg must
+ # only be initialized once. Projects that initialize FFmpeg externally, such
+ # as Chromium, must turn this flag off so that WebRTC does not also
+ # initialize.
+ rtc_initialize_ffmpeg = !build_with_chromium
}
# A second declare_args block, so that declarations within it can
diff --git a/webrtc/modules/video_coding/BUILD.gn b/webrtc/modules/video_coding/BUILD.gn
index 115ed35..2d9e08d 100644
--- a/webrtc/modules/video_coding/BUILD.gn
+++ b/webrtc/modules/video_coding/BUILD.gn
@@ -140,6 +140,9 @@
if (rtc_use_h264) {
defines += [ "WEBRTC_THIRD_PARTY_H264" ]
+ if (rtc_initialize_ffmpeg) {
+ defines += [ "WEBRTC_INITIALIZE_FFMPEG" ]
+ }
sources += [
"codecs/h264/h264_decoder_impl.cc",
"codecs/h264/h264_decoder_impl.h",
diff --git a/webrtc/modules/video_coding/codecs/h264/h264.gypi b/webrtc/modules/video_coding/codecs/h264/h264.gypi
index 0e80561..e82b0fd 100644
--- a/webrtc/modules/video_coding/codecs/h264/h264.gypi
+++ b/webrtc/modules/video_coding/codecs/h264/h264.gypi
@@ -27,6 +27,13 @@
'defines': [
'WEBRTC_THIRD_PARTY_H264',
],
+ 'conditions': [
+ ['rtc_initialize_ffmpeg==1', {
+ 'defines': [
+ 'WEBRTC_INITIALIZE_FFMPEG',
+ ],
+ }],
+ ],
'dependencies': [
'<(DEPTH)/third_party/ffmpeg/ffmpeg.gyp:ffmpeg',
'<(DEPTH)/third_party/openh264/openh264.gyp:openh264_encoder',
diff --git a/webrtc/modules/video_coding/codecs/h264/h264_decoder_impl.cc b/webrtc/modules/video_coding/codecs/h264/h264_decoder_impl.cc
index 75d2bfa..ea96f25 100644
--- a/webrtc/modules/video_coding/codecs/h264/h264_decoder_impl.cc
+++ b/webrtc/modules/video_coding/codecs/h264/h264_decoder_impl.cc
@@ -34,8 +34,9 @@
const size_t kUPlaneIndex = 1;
const size_t kVPlaneIndex = 2;
-#if !defined(WEBRTC_CHROMIUM_BUILD)
+#if defined(WEBRTC_INITIALIZE_FFMPEG)
+rtc::CriticalSection ffmpeg_init_lock;
bool ffmpeg_initialized = false;
// Called by FFmpeg to do mutex operations if initialized using
@@ -61,10 +62,8 @@
return -1;
}
-// TODO(hbos): Assumed to be called on a single thread. Should DCHECK that
-// InitializeFFmpeg is only called on one thread or make it thread safe.
-// See https://bugs.chromium.org/p/webrtc/issues/detail?id=5427.
void InitializeFFmpeg() {
+ rtc::CritScope cs(&ffmpeg_init_lock);
if (!ffmpeg_initialized) {
if (av_lockmgr_register(LockManagerOperation) < 0) {
RTC_NOTREACHED() << "av_lockmgr_register failed.";
@@ -75,7 +74,7 @@
}
}
-#endif // !defined(WEBRTC_CHROMIUM_BUILD)
+#endif // defined(WEBRTC_INITIALIZE_FFMPEG)
// Called by FFmpeg when it is done with a frame buffer, see AVGetBuffer2.
void AVFreeBuffer2(void* opaque, uint8_t* data) {
@@ -179,13 +178,15 @@
return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
}
- // In Chromium FFmpeg will be initialized outside of WebRTC and we should not
- // attempt to do so ourselves or it will be initialized twice.
- // TODO(hbos): Put behind a different flag in case non-chromium project wants
- // to initialize externally.
- // See https://bugs.chromium.org/p/webrtc/issues/detail?id=5427.
-#if !defined(WEBRTC_CHROMIUM_BUILD)
- // Make sure FFmpeg has been initialized.
+ // FFmpeg must have been initialized (with |av_lockmgr_register| and
+ // |av_register_all|) before we proceed. |InitializeFFmpeg| does this, which
+ // makes sense for WebRTC standalone. In other cases, such as Chromium, FFmpeg
+ // is initialized externally and calling |InitializeFFmpeg| would be
+ // thread-unsafe and result in FFmpeg being initialized twice, which could
+ // break other FFmpeg usage. See the |rtc_initialize_ffmpeg| flag.
+#if defined(WEBRTC_INITIALIZE_FFMPEG)
+ // Make sure FFmpeg has been initialized. Subsequent |InitializeFFmpeg| calls
+ // do nothing.
InitializeFFmpeg();
#endif