Closed Bug 987979 Opened 10 years ago Closed 10 years ago

Update WebRTC code to webrtc.org stable branch 3.50

Categories

(Core :: WebRTC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jesup, Assigned: jesup)

References

Details

Attachments

(10 files)

webrtc.org stable branch 3.50 is being used in Chrome 34, to be released soon.

There is no newer stable branch.

Process is being documented here:
https://wiki.mozilla.org/Media/WebRTC/Updating_Process
Green try on desktop (Android/B2G sections not merged yet)
https://tbpl.mozilla.org/?tree=Try&rev=ad14572d8f6d
Assignee: nobody → rjesup
Most of this is forward porting existing code.
Attachment #8428690 - Flags: review?(blassey.bugs)
Same, forward porting existing code.
Attachment #8428692 - Flags: review?(blassey.bugs)
Do we still need this? Probably not as we bumped to platform-17 very recently?
Attachment #8428693 - Flags: review?(blassey.bugs)
Attachment #8428699 - Flags: review?(blassey.bugs)
Comment on attachment 8428685 [details] [diff] [review]
Patch 1. Various build fixes

Review of attachment 8428685 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nits

::: media/webrtc/signaling/signaling.gyp
@@ +167,5 @@
>          '_NO_LOG4CXX',
>          'USE_SSLEAY',
>          '_CPR_USE_EXTERNAL_LOGGER',
>          'WEBRTC_RELATIVE_PATH',
> +        'HAVE_WEBRTC_VIDEO',

No change, remove (unless maybe it's a tab-removal)

@@ +220,5 @@
>              '-I$(ANDROID_SOURCE)/frameworks/native/include',
>              '-I$(ANDROID_SOURCE)/frameworks/native/opengl/include',
>            ],
>            'defines' : [
> +            'MOZ_WEBRTC_OMX',

no change (tabs?)

::: media/webrtc/trunk/build/all.gyp
@@ -103,5 @@
>                ],
>              }],
>            ],
>          }],
> -        ['toolkit_uses_gtk==1', {

Instead of removing it, add a test against build_with_mozilla==0 (and maybe comment why)

::: media/webrtc/trunk/build/filename_rules.gypi
@@ -65,5 @@
>          ['exclude', '_(x|x11)(_unittest)?\\.(h|cc)$'],
>          ['exclude', '(^|/)x11_[^/]*\\.(h|cc)$'],
>        ],
>      }],
> -    ['<(toolkit_uses_gtk)!=1 or >(nacl_untrusted_build)==1', {

Instead, build_with_mozilla==0 (and maybe comment why)
Attachment #8428685 - Flags: review?(rjesup) → review+
Comment on attachment 8428686 [details] [diff] [review]
Patch 2. Android Audio related fixes

Review of attachment 8428686 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/trunk/webrtc/modules/audio_device/android/audio_record_jni.h
@@ +111,5 @@
>    int32_t SetRecordingSampleRate(const uint32_t samplesPerSec);
>  
> +  static const uint32_t N_REC_SAMPLES_PER_SEC = 16000; // Default is 16 kHz
> +  static const uint32_t N_REC_CHANNELS = 1; // default is mono recording
> +  static const uint32_t REC_BUF_SIZE_IN_SAMPLES = 480; // Handle max 10 ms @ 48 kHz

Ok, but we need to be careful if we pass these as arguments to functions (to avoid needing to do tricks like "+N_REC_CHANNELS" to force it to be referenceable).  If it builds clean on all platforms, we're good.
Attachment #8428686 - Flags: review?(rjesup) → review+
Comment on attachment 8428687 [details] [diff] [review]
Patch 3. OpenSLES dummy backend, fixes

Review of attachment 8428687 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with use_openssl moved

::: media/webrtc/trunk/build/common.gypi
@@ +1049,5 @@
>          # Uses Android's crash report system
>          'linux_breakpad%': 0,
>  
>          # Always uses openssl.
> +        'use_openssl%': 0,

Move to build/gyp.mozbuild:
    use_openssl: 0,

::: media/webrtc/trunk/supplement/supplement.gypi
@@ +9,5 @@
>      'include_internal_audio_device%': 1,
>      'include_internal_video_capture%': 1,
>      'include_internal_video_render%': 1,
>      'include_pulse_audio%': 1,
> +    'use_openssl%': 0,

gyp.mozbuild will override this

::: media/webrtc/trunk/webrtc/modules/audio_device/android/opensles_output.h
@@ +267,5 @@
> +// Dummy OpenSlesOutput
> +class OpenSlesOutput : public PlayoutDelayProvider {
> + public:
> +  explicit OpenSlesOutput(const int32_t id) : 
> +    initialized_(false), speaker_initialized_(false), 

trailing spaces

::: media/webrtc/trunk/webrtc/modules/audio_device/audio_device.gypi
@@ -154,5 @@
>                'conditions': [
>                  ['enable_android_opensl==1', {
>                    'sources': [
> -                    'opensl/audio_device_opensles.cc',
> -                    'opensl/audio_device_opensles.h',

Are any of these files removed?  I see 3 (empty) files removed.  Did these just go elsewhere?
Attachment #8428687 - Flags: review?(rjesup) → review+
Attachment #8428688 - Flags: review?(blassey.bugs) → review+
Comment on attachment 8428688 [details] [diff] [review]
Patch 4. Include and build NDK cpu-features

Review of attachment 8428688 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/trunk/webrtc/system_wrappers/source/droid-cpu-features.c
@@ +275,5 @@
> +            archNumber = strtol(cpuArch, &end, 10);
> +
> +            /* Here we assume that ARMv8 will be upwards compatible with v7
> +                * in the future. Unfortunately, there is no 'Features' field to
> +                * indicate that Thumb-2 is supported.

ARM1156 (which is ARMv6) introduced Thumb-2.
Comment on attachment 8428688 [details] [diff] [review]
Patch 4. Include and build NDK cpu-features

Review of attachment 8428688 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/trunk/webrtc/system_wrappers/source/droid-cpu-features.c
@@ +275,5 @@
> +            archNumber = strtol(cpuArch, &end, 10);
> +
> +            /* Here we assume that ARMv8 will be upwards compatible with v7
> +                * in the future. Unfortunately, there is no 'Features' field to
> +                * indicate that Thumb-2 is supported.

Upon further review, the ARMv6 chips that support Thumb-2 don't support the entire instruction set. Better to restrict to ARMv7 and higher.

http://en.wikipedia.org/wiki/ARM_Cortex-M0#Instruction_sets
Attachment #8428689 - Flags: review?(blassey.bugs) → review+
Attachment #8428690 - Flags: review?(blassey.bugs) → review+
Attachment #8428692 - Flags: review?(blassey.bugs) → review+
Attachment #8428693 - Flags: review?(blassey.bugs) → review+
Attachment #8428698 - Flags: review?(blassey.bugs) → review+
Attachment #8428699 - Flags: review?(blassey.bugs) → review+
Green Try: (not waiting on Mac 10.8 or 2.2/2.3 runs which haven't even started yet)
https://tbpl.mozilla.org/?tree=Try&rev=aaa2a429dc5c
I've added a note to our Mercurial documentation on how to properly set up qcrecord so it doesn't lose patch authorship information :-/
Depends on: 1018928
Blocks: 1109248
This bug added a non-existent symlink (media/webrtc/trunk/webrtc/tools/e2e_quality/audio/perf) which interfered with grep.  I removed it, hope that's ok.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca8b92f0266
You need to log in before you can comment on or make changes to this bug.