818843 - Enable the makefile system to build WebRTC on B2G.

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[StevenLee[:slee]]

Enable the makefile system to build WebRTC on B2G.

Comment 1

[StevenLee[:slee]]

Attachment: part 1, configure.in - WIP

Comment 2

[StevenLee[:slee]]

Attachment: part 2, network

Comment 3

[StevenLee[:slee]]

Attachment: part 3, media - WIP

Comment 4

[StevenLee[:slee]]

I could build WebRTC on B2G and I could run the media, {fake:true, video:true}.

Comment 5

[StevenLee[:slee]]

Attachment: test patch: enable MOZ_MEDIA_NAVIGATOR on B2G

Enable MOZ_MEDIA_NAVIGATOR and turn on preference on B2G

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Wow!  Great work Steven!  I'll look over these patches today.

Comment 7

[StevenLee[:slee]]

After updating to the latest code, build fails. Bug 750869 defined variable, target_arch. If the target_arch is "arm" and is not armv7, there are building problems. That's because "arm_neon" is only meaningful on armv7, [1]. But in the build system checks "arm_neon" without caring about the value of armv7, [2]. I will file a new bug to solve this problem first.

[1] https://mxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/build/common.gypi#192
[2] https://mxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/src/common_audio/signal_processing/signal_processing.gypi#78

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to StevenLee from comment #7)
> After updating to the latest code, build fails. Bug 750869 defined variable,
> target_arch. If the target_arch is "arm" and is not armv7, there are
> building problems. That's because "arm_neon" is only meaningful on armv7,
> [1]. But in the build system checks "arm_neon" without caring about the
> value of armv7, [2]. I will file a new bug to solve this problem first.

I suspect this is related to, or even fixed by, the patch in bug 815883.

Comment 9

[StevenLee[:slee]]

(In reply to Dan Mosedale (:dmose) from comment #8)
> I suspect this is related to, or even fixed by, the patch in bug 815883.
Hi dmose,

I think maybe we are fixing different problems. The problem I encountered is that the checking logic has problem. In [1], the build system will include some unnecessary files and they cause symbols conflicts when linking. The patch of bug 815883 tries to fix some dependencies and flags for arm_neon but it does not set the value of "arm_neon". The default value of "arm_neon" is true. So that if we don't fix the checking logic(that is fixed in 819246) and build WebRTC on ARM, we still get symbol conflict problem, it that right?

[1] https://mxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/src/common_audio/signal_processing/signal_processing.gypi#78

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Sounds right to me, though symbol conflicts isn't exactly what I would expect to be the failure mode for trying to use ARM NEON instructions when targeting ARMv6.

Comment 11

[StevenLee[:slee]]

Attachment: part 1, configure.in

Hi jesup,

Would you please review my patches? Thanks.

In this patch, I 
1. Enable WEBRTC on B2G
2. Add new definition, moz_widget_toolkit_gonk, that will be enabled on gonk.
3. It does not build gtest.gyp
4. Include new paths for alsa and rpc

Comment 12

[StevenLee[:slee]]

Attachment: Part 2, network

1. Add new definition, __Userspace_os_Gonk.
2. Add 2 new functions, get_interface getifaddrs and freeifaddrs, these functions are borrowed from android. I am not sure if it is suitable to do this or we can have better solution.
3. Fix some platform dependency include.

Comment 13

[StevenLee[:slee]]

Hi jesup,

I tried B2G unagi and Linux. They are both fine. But when I upload my patches to try server, there are building errors. I will fix these errors first then update new patches. I will cancel the reviews.

Comment 14

[StevenLee[:slee]]

Attachment: part 1, configure.in

1. Enable WebRTC on B2G
2. Don't link OpenSLES. B2G process crashes when linking this library.
3. Add moz_widget_toolkit_gonk
4. Disable testing code.

Comment 15

[StevenLee[:slee]]

Attachment: Part 2, network

1. Add __Userspace_os_Gonk

Comment 16

[StevenLee[:slee]]

Attachment: part 3, media

1. disable testing code in mtransport
2. add WEBRTC_GONK 
3. fix some errors of include headers.
4. add some wrapper functions of msg

Comment 17

[StevenLee[:slee]]

Hi jesup,

Above are my updated patches. I fixed the compiling error of emulator and pandaboards. 
Here is the try server log. Please check it.
* try: -b do -p all -u none -t none
** https://tbpl.mozilla.org/?tree=Try&rev=33a1c396e97b

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 692740 [details] [diff] [review]
part 1, configure.in

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

r+ with nits fixed as needed

::: configure.in
@@ +5255,5 @@
> +            MOZ_WEBRTC=1
> +        else
> +            dnl Make sure doesn't get matched by *-linux*
> +            MOZ_WEBRTC=
> +        fi

Likely will get a merge conflict with the android code.  I suggest rebasing your patches on top of the current set of Android patches from gcp/dmose to minimize bitrot and merge issues.

@@ +9052,5 @@
>  dnl so that regeneration via dependencies works correctly
> +   if test -n HAVE_CLOCK_MONOTONIC; then
> +      WEBRTC_CONFIG="-D build_with_mozilla=1 -D moz_have_clock_monotonic=1 --include ${srcdir}/media/webrtc/webrtc_config.gypi -D FORCED_INCLUDE_FILE=${srcdir}/media/webrtc/webrtc_config.gypi"
> +   else
> +      WEBRTC_CONFIG="-D build_with_mozilla=1 -D moz_have_clock_monotonic=0 --include ${srcdir}/media/webrtc/webrtc_config.gypi -D FORCED_INCLUDE_FILE=${srcdir}/media/webrtc/webrtc_config.gypi"

I'd prefer to append "-D moz_have_clock_monotonic=X" to the WEBRTC_CONFIG var after it's set, or do it in a single line in some fashion.  It's painful and error-inducing to have two copies of the same code/defines.

@@ +9086,5 @@
> +         --generator-output=${_objdir}/media/webrtc/signalingtest \
> +         ${srcdir}/media/webrtc/signaling/signaling.gyp
> +       if test "$?" != 0; then
> +         AC_MSG_ERROR([failed to generate WebRTC/SignalingTest Makefiles])
> +       fi

question: do we need to disable creating the makefiles, or just disable building them?  If it's just building, disable building the tests from the Makefile.in in (IIRC) toolkit.

@@ +9099,5 @@
> +       --generator-output=${_objdir}/media/webrtc/trunk/testing/ \
> +       ${srcdir}/media/webrtc/trunk/testing/gtest.gyp
> +     if test "$?" != 0; then
> +       AC_MSG_ERROR([failed to generate gtest Makefiles])
> +     fi

Ditto

Comment 19

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to Randell Jesup [:jesup] from comment #18)

> ::: configure.in
> @@ +5255,5 @@
> > +            MOZ_WEBRTC=1
> > +        else
> > +            dnl Make sure doesn't get matched by *-linux*
> > +            MOZ_WEBRTC=
> > +        fi
> 
> Likely will get a merge conflict with the android code.  I suggest rebasing
> your patches on top of the current set of Android patches from gcp/dmose to
> minimize bitrot and merge issues.

The Android build patches have all landed, so as long as this patch is based on mozilla-central or mozilla-inbound from yesterday or newer, that shouldn't be a problem.  Of course, trying an Android build with --enable-webrtc to be sure that these changes don't actually cause that Fennec configuration not to build would be nice.

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 692742 [details] [diff] [review]
Part 2, network

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

r+, but really this likely would be better using os_Linux and then using a few #if GONK (or whatever) like Android is doing.  See dmose/gcp's patch for guidance.  I'd prefer to follow their lead in this module (and the diffs will likely be smaller too)

::: netwerk/sctp/src/netinet/sctp.h
@@ +37,5 @@
>  
>  #ifndef _NETINET_SCTP_H_
>  #define _NETINET_SCTP_H_
>  
> +#if (defined(__APPLE__) || defined(__Userspace_os_Linux) || defined(__Userspace_os_Darwin)) || defined(__Userspace_os_Gonk)

Probably better would be !__Userspace_os_Windows  - I presume FreeBSD has stdint too

::: netwerk/sctp/src/user_ip6_var.h
@@ +88,5 @@
>  #define s6_addr16 u.Word
>  #endif
>  #if !defined(__Userspace_os_Windows)
>  #if !defined(__Userspace_os_Linux)
> +#if !defined(__Userspace_os_Gonk)

Probably combine into one #if

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 692744 [details] [diff] [review]
part 3, media

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

Overall good, but I'm mildly concerned that there are many android optimizations in media/webrtc/trunk that should also apply to B2G (some of which relate to the processor, but others relate to the likely horsepower of the CPU, etc).  However, there are also a bunch of userspace-related Android code bits in there (video renedering, etc), so without reviewing all the code that references ANDROID in that dir, I can't say which way is preferred.

Please also compare to changes in this area currently being used by the Android team

::: media/webrtc/Makefile.in
@@ +20,3 @@
>    trunk/testing \
> +	$(NULL)
> +endif

Correct indentation

::: media/webrtc/signaling/signaling.gyp
@@ +20,5 @@
> +          'WEBRTC_GONK',
> +       ],
> +      }],
> +    ],
> +  }, 

remove trailing spaces

::: media/webrtc/trunk/src/build/common.gypi
@@ +124,5 @@
> +        'defines' : [
> +          'WEBRTC_GONK',
> +        ],
> +      }],
> +  

trailing whitespace

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 692742 [details] [diff] [review]
Part 2, network

On second thought - r- for now; I'd really prefer (if it works) to use the approach that our Android team is taking.  Please ping dmose if you have any questions.

Comment 23

[StevenLee[:slee]]

Attachment: part 1, configure.in - v2

1. Address comment 18. It creates the makefiles for test cases and disable building them from toolkit/toolkit-makefiles.sh.
2. The network patch is no need when I update to the latest code.

Comment 24

[StevenLee[:slee]]

Attachment: part 2 - media - v2

Address comment 21. 

Here is the try server log. 
* try: -b do -p all -u all -t none
** https://tbpl.mozilla.org/?tree=Try&rev=25607b1471b1

Comment 25

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 697760 [details] [diff] [review]
part 1, configure.in - v2

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

::: configure.in
@@ +5261,5 @@
>  if test -n "$MOZ_WEBRTC"; then
>      case "$target" in
>      *-android*|*-linuxandroid*)
> +        if test -n "$MOZ_B2G"; then
> +            MOZ_WEBRTC=1

You really want to turn WebRTC on for all gonk builds, even in it's current state?  (Not that that's necessarily wrong, I just wanted to be sure that it's intentional).

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f5b253ab7b7
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9831bed6bb7

Comment 27

[StevenLee[:slee]]

(In reply to Dan Mosedale (:dmose) from comment #25)
> You really want to turn WebRTC on for all gonk builds, even in it's current
> state?  (Not that that's necessarily wrong, I just wanted to be sure that
> it's intentional).
Hi Dan,

Thanks for your reminding. I think that the patch just enables building webrtc code. The code is not accessible except that we turn on MOZ_MEDIA_NAVIGATOR and media.navigator.enabled. So that I think it should not effect the stability of B2G.

Comment 28

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6f5b253ab7b7
https://hg.mozilla.org/mozilla-central/rev/c9831bed6bb7

Comment 29

[Doug Turner (:dougt)]

This broke building B2G on the mac.

/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:180: error: 'timebase' was not declared in this scope/Users/dougt/builds/mozilla-central/me\
dia/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:184: error: 'KERN_SUCCESS' was not declared in this scope/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/\
../system_wrappers/interface/tick_util.h:180: error: 'timebase' was not declared in this scope


/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:183: error: 'kern_return_t' was not declared in this scope/Users/dougt/builds/mozilla-centr\
al/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:183: error: 'kern_return_t' was not declared in this scope

/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:183: error: expected ';' before 'retval'/Users/dougt/builds/mozilla-central/media/webrtc/tr\
unk/src/video_engine/../system_wrappers/interface/tick_util.h:183: error: expected ';' before 'retval'

/Users/dougt/builds/m/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:191: error: 'mach_absolute_time' was not declared in this scopemake[5]\
: 
ozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:184: error: 'retval' was not declared in this scope*** [vie_base_impl.o] Error 1

/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:184: error: 'KERN_SUCCESS' was not declared in this scope/Users/dougt/builds/mozilla-centra\
l/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:191: error: 'timebase' was not declared in this scope

make[5]: *** Waiting for unfinished jobs....
/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:184: error: 'retval' was not declared in this scope
/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:184
: error: 'KERN_SUCCESS' was not declared in this scope/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:191: error: 'mach_absolute_time' was \
not declared in this scope

/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:191: error: 'timebase' was not declared in this scope
/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:191: error: 'mach_absolute_time' was not declared in this scope
/Users/dougt/builds/mozilla-central/media/webrtc/trunk/src/video_engine/../system_wrappers/interface/tick_util.h:191: error: 'timebase' was not declared in this scope



Sorry - Backed out:

  https://hg.mozilla.org/mozilla-central/rev/a2c05e82dd89
  https://hg.mozilla.org/mozilla-central/rev/8155dd7632bb

Comment 30

[StevenLee[:slee]]

(In reply to Doug Turner (:dougt) from comment #29)
> This broke building B2G on the mac.
Hi Doug,

Did you build B2G desktop for MAC? Could you give your os version?
[1] uses the similar code and it can be built. Do I miss something?
Thanks.

[1] https://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/time_mac.cc#90

Comment 31

[StevenLee[:slee]]

Attachment: part 2 - media - v3

rebase

Comment 32

[StevenLee[:slee]]

(In reply to StevenLee from comment #30)
> [1]
> https://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/
> time_mac.cc#90
This code could be compile on mac. 
Here is the try server log. https://tbpl.mozilla.org/?tree=Try&rev=c028a0bcdd95

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8aa49207d1c
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b803c2821b9

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e8aa49207d1c
https://hg.mozilla.org/mozilla-central/rev/9b803c2821b9

Comment 35

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Were the configure.in changes reviewed by a build peer?

Comment 36

[Justin Lebar (not reading bugmail)]

Steven, this is still broken on mac.  I filed bug 834859.

The tryserver log you point to does not show the build working on mac.  What is broken is building B2G for a device (e.g. Otoro) on Mac.  Tryserver does not test this configuration.  I filed bug 834890 on this.

Comment 37

[Justin Lebar (not reading bugmail)]

I'm testing that my backout actually works.  If so, I'll push it.

Comment 38

[Randell Jesup [:jesup] (needinfo me)]

Also, before pushing again please get review from ted (:ted).  All significant makesystem changes (especially configure.in) need build peer review, including changes to .gyp files.  Most of these are straightforward, but it's still required (and can catch problems like this backout).

Comment 39

[Justin Lebar (not reading bugmail)]

Backed out, sorry.

Please note that the error in comment 29 was not the only one I saw.  It also appeared that we were trying to build Mac-specific components in this cross-compiled build:

> src/media/webrtc/trunk/src/modules/audio_device/main/source/mac/audio_mixer_manager_mac.h:25: error: 'AudioDeviceID' has not been declared

https://hg.mozilla.org/integration/mozilla-inbound/rev/38c61e9826b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2a2c71ce437

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/38c61e9826b2
https://hg.mozilla.org/mozilla-central/rev/e8aa49207d1c

Comment 41

[StevenLee[:slee]]

Hi Justin,

Sorry for the inconvenience. 
I will borrow a Mac and fix the compiling error.

Hi Ms2ger,
I will ask ted for review my patch when I work out a new version.

Comment 42

[StevenLee[:slee]]

Attachment: part 1, configure.in - v3

Comment 43

[StevenLee[:slee]]

Attachment: part 2 - media - v4

Comment 44

[StevenLee[:slee]]

Attachment: part 3, do not build gtest - v1

Here is the try server log. https://tbpl.mozilla.org/?tree=Try&rev=e0ae24cf8aa4
I try to build the opt version of all platforms and test Android-Only Unittest Suites. There is one fault, [uploadsymbols] Error 1. It's related to bug 847683.

Comment 45

[StevenLee[:slee]]

BTW, I could also build Fennec, FireFox, and FFOS on Mac with these patches applied.

Comment 46

[Joe Cheng [:jcheng] (please needinfo)]

wonder if Ted got some time to review this?

Comment 47

[StevenLee[:slee]]

Attachment: part 4 - include peerconnection to b2g package-manifest

Comment 48

[(not currently active) Ted Mielczarek]

Comment on attachment 721013 [details] [diff] [review]
part 1, configure.in - v3

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

::: configure.in
@@ +9042,5 @@
> +   if test "${MOZ_WIDGET_TOOLKIT}" != "gonk"; then
> +      EXTRA_GYP_DEFINES="-D gtest_target_type=executable -D android_toolchain=${android_toolchain} -G os=android "
> +   else
> +      EXTRA_GYP_DEFINES="-G os=linux "
> +   fi

I would swap these two branches around so that your test is MOZ_WIDGET_TOOLKIT = gonk.

@@ +9062,5 @@
>  
> +   if test "${MOZ_WIDGET_TOOLKIT}" = "gonk"; then
> +      EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D moz_widget_toolkit_gonk=1"
> +   else
> +      EXTRA_GYP_DEFINES="${EXTRA_GYP_DEFINES} -D moz_widget_toolkit_gonk=0"

This is a little kludgy. Could we add moz_widget_toolkit_gonk=0 as a default in a gypfile somewhere?

Comment 49

[(not currently active) Ted Mielczarek]

Comment on attachment 721017 [details] [diff] [review]
part 2 - media - v4

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

#media suggested that ehugg review the cpr changes. I'm only reviewing the gyp changes.

::: media/webrtc/trunk/build/common.gypi
@@ +401,5 @@
>  
>        'conditions': [
> +         ['moz_widget_toolkit_gonk==1', {
> +           'variables': {
> +             'disable_sse2': 1,

Shouldn't this be covered by the
 ['target_arch=="arm" or target_arch=="mipsel"', {
block already in this file?

::: media/webrtc/trunk/webrtc/build/common.gypi
@@ +224,5 @@
>          ],
>        }],
>        ['OS=="linux"', {
> +        'conditions': [
> +          ['moz_have_clock_monotonic==1', {

Seems like this doesn't really need a moz prefix, it's a pretty generic thing.

Comment 50

[(not currently active) Ted Mielczarek]

Comment on attachment 721020 [details] [diff] [review]
part 3, do not build gtest - v1

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

r- for the toolkit.mozbuild piece, but otherwise looks ok with a few nits.

::: media/webrtc/moz.build
@@ +9,3 @@
>  ]
>  
> +if not CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gonk':

You want to say CONFIG['MOZ_WIDGET_TOOLKIT'] != 'gonk':
(protip: moz.build files are just Python)

::: toolkit/toolkit.mozbuild
@@ +136,5 @@
>      add_tier_dir('platform', 'testing/specialpowers')
>  
>  if CONFIG['MOZ_ENABLE_GTEST']:
> +    if CONFIG['MOZ_WIDGET_TOOLKIT'] != 'gonk':
> +        add_tier_dir('platform', 'testing/gtest')

This is not the right way to do this. You should fix configure to not enable MOZ_ENABLE_GTEST for gonk until we can figure out how to make it build. (and I'd appreciate a followup on making it build)

@@ +249,5 @@
>          'testing/modules',
>      ])
>  
>      if CONFIG['MOZ_WEBRTC']:
> +        if CONFIG['MOZ_WIDGET_TOOLKIT'] != 'gonk':

You can just and this into the if CONFIG['MOZ_WEBRTC'] instead of making a nested if.

Comment 51

[Ethan Hugg [:ehugg]]

Comment on attachment 721017 [details] [diff] [review]
part 2 - media - v4

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

Good to see you were able to re-use the Linux version of CPR instead of duping another one.

::: media/mtransport/third_party/nICEr/nicer.gyp
@@ +206,5 @@
>  		 ],
> +              }],
> +              ['moz_widget_toolkit_gonk==1', {
> +                'defines' : [
> +                  'WEBRTC_GONK',

I didn't see an instance of this define being used in mtransport, is there another patch that uses it?

::: media/webrtc/trunk/webrtc/system_wrappers/source/thread_posix.cc
@@ +315,5 @@
>      CriticalSectionScoped cs(crit_state_);
>      alive_ = true;
>      dead_  = false;
>    }
> +#if (defined(WEBRTC_LINUX) || defined(WEBRTC_ANDROID) || defined(WEBRTC_GONK)) 

We generally avoid adding trailing whitespace to this code.

Comment 52

[StevenLee[:slee]]

Attachment: part 1, configure.in - v4

1. Fixed nits mentioned in comment 48.
2. change moz_have_clock_monotonic to have_clock_monotonic

Comment 53

[StevenLee[:slee]]

Attachment: part 2 - media - v5

1. Fix nits in comment 49 and comment 51
2. Add definition, MOZ_MEDIA_NAVIGATOR=1, in b2g/confvars.sh. Sorry for that I did not put it in the previous patch.
3. moz_widget_toolkit_gonk has default value 0
4. set "include_internal_audio_device" in configure.in. In the previous version, it is set in supplement.gypi. This flag should be removed when we are porting audio devices. 

I have new changes. Could you review it again? Thanks.

Comment 54

[StevenLee[:slee]]

Attachment: part 3, do not build gtest - v2

Fix the nits in comment 50.

Comment 55

[StevenLee[:slee]]

Attachment: part 4 - include peerconnection to b2g package-manifest

Add reviewer information

Comment 56

[StevenLee[:slee]]

Here is the result on try server:

https://tbpl.mozilla.org/?tree=Try&rev=80c39b966814

Comment 57

[(not currently active) Ted Mielczarek]

Comment on attachment 725334 [details] [diff] [review]
part 2 - media - v5

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

::: media/webrtc/trunk/build/common.gypi
@@ +398,5 @@
>  
>        'sas_dll_path%': '<(DEPTH)/third_party/platformsdk_win7/files/redist/x86',
>        'wix_path%': '<(DEPTH)/third_party/wix',
>  
> +       'conditions': [

Did you accidentally indent this by a space?

Comment 58

[StevenLee[:slee]]

Attachment: part 2 - media - v6

addressed comment 57

Comment 59

[Ryan VanderMeulen [:RyanVM]]

Patch 1 doesn't apply cleanly. Please rebase.

Comment 60

[StevenLee[:slee]]

Attachment: part 1 - configure.in - v5

rebase

Comment 61

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8b5d339bb1b
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7bd912ce45a
https://hg.mozilla.org/integration/mozilla-inbound/rev/3491ce05b207
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6b9b196436f

Comment 62

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d8b5d339bb1b
https://hg.mozilla.org/mozilla-central/rev/b7bd912ce45a
https://hg.mozilla.org/mozilla-central/rev/3491ce05b207
https://hg.mozilla.org/mozilla-central/rev/e6b9b196436f