diff --git a/webrtc/build/common.gypi b/webrtc/build/common.gypi index a77420c070..d5b79f9c13 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 f784ab1830..886b899543 100644 --- a/webrtc/build/webrtc.gni +++ b/webrtc/build/webrtc.gni @@ -96,9 +96,17 @@ declare_args() { # 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 115ed3588b..2d9e08d347 100644 --- a/webrtc/modules/video_coding/BUILD.gn +++ b/webrtc/modules/video_coding/BUILD.gn @@ -140,6 +140,9 @@ source_set("webrtc_h264") { 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 0e80561bdf..e82b0fd4cd 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 75d2bfa732..ea96f25a1e 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 kYPlaneIndex = 0; 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 @@ int LockManagerOperation(void** lock, AVLockOp op) 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 @@ void InitializeFFmpeg() { } } -#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 @@ int32_t H264DecoderImpl::InitDecode(const VideoCodec* codec_settings, 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