Update webrtc.org code from stable branch 3.12

RESOLVED FIXED in mozilla18

Status

()

Core
WebRTC
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

(Blocks: 1 bug)

Trunk
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(3 attachments)

(Assignee)

Description

6 years ago
Update media/webrtc/trunk from webrtc.org 3.12.  Includes all rolled up changes to avoid bloating the hg database via pull.  Previously running on alder. Current rev on that branch we're importing here is 2820.

Try runs (second fixes the windows issue in the first)
https://tbpl.mozilla.org/?tree=Try&rev=e4c63f0489a0
https://tbpl.mozilla.org/?tree=Try&rev=69ed7d14f3ed
(Assignee)

Comment 1

6 years ago
Created attachment 667789 [details] [diff] [review]
cleanup from importing webrtc.org update.  Part is bug 778801 (
(Assignee)

Comment 2

6 years ago
Comment on attachment 667789 [details] [diff] [review]
cleanup from importing webrtc.org update.  Part is bug 778801 (

The changes here are almost all rolled-up build merges from alder, plus bug 778801 (r=derf)
Attachment #667789 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 3

6 years ago
Created attachment 667887 [details] [diff] [review]
media/webrtc/trunk changes (minus Opus/etc)

These are changes to alder's media/webrtc/trunk after the merge to m-c of rev 2047 plus our changes to that point.  Removed are the separately reviewed Opus/etc changes, and bug 784082 and bug 772201 (both r+ ted).  This are effectively all makesystem or build compat changes (VS2012, etc).
Attachment #667887 - Flags: review?(ted.mielczarek)
Comment on attachment 667789 [details] [diff] [review]
cleanup from importing webrtc.org update.  Part is bug 778801 (

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

I'd like to run this by glandium since I wrote some of the makefile changes here.

::: configure.in
@@ +2984,5 @@
> +MOZ_CHECK_HEADERS([linux/if_addr.h linux/rtnetlink.h],,,[#include <sys/socket.h>])
> +
> +dnl Try for MMX support
> +dnl NB - later gcc versions require -mmmx for this header to be successfully
> +dnl included (or another option which implies it, such as -march=pentium-mmx)

Does this note mean we have to add this compiler option to make this actually work?

::: toolkit/library/Makefile.in
@@ +385,5 @@
> +#EXTRA_DSO_LDOPTS += \
> +#  $(DEPTH)/media/mtransport/build/$(LIB_PREFIX)mtransport.$(LIB_SUFFIX) \
> +#  $(DEPTH)/media/webrtc/signaling/signaling_ecc/$(LIB_PREFIX)ecc.$(LIB_SUFFIX) \
> +#  $(DEPTH)/media/webrtc/signaling/signaling_sipcc/$(LIB_PREFIX)sipcc.$(LIB_SUFFIX) \
> +#  $(NULL)

If this isn't needed let's just remove it.
Attachment #667789 - Flags: review?(ted.mielczarek)
Attachment #667789 - Flags: review?(mh+mozilla)
Attachment #667789 - Flags: review+
Attachment #667887 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 5

6 years ago
(In reply to Ted Mielczarek [:ted] from comment #4)
> Comment on attachment 667789 [details] [diff] [review]
> cleanup from importing webrtc.org update.  Part is bug 778801 (
> 
> Review of attachment 667789 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'd like to run this by glandium since I wrote some of the makefile changes
> here.
> 
> ::: configure.in
> @@ +2984,5 @@
> > +MOZ_CHECK_HEADERS([linux/if_addr.h linux/rtnetlink.h],,,[#include <sys/socket.h>])
> > +
> > +dnl Try for MMX support
> > +dnl NB - later gcc versions require -mmmx for this header to be successfully
> > +dnl included (or another option which implies it, such as -march=pentium-mmx)
> 
> Does this note mean we have to add this compiler option to make this
> actually work?

> 
> ::: toolkit/library/Makefile.in
> @@ +385,5 @@
> > +#EXTRA_DSO_LDOPTS += \
> > +#  $(DEPTH)/media/mtransport/build/$(LIB_PREFIX)mtransport.$(LIB_SUFFIX) \
> > +#  $(DEPTH)/media/webrtc/signaling/signaling_ecc/$(LIB_PREFIX)ecc.$(LIB_SUFFIX) \
> > +#  $(DEPTH)/media/webrtc/signaling/signaling_sipcc/$(LIB_PREFIX)sipcc.$(LIB_SUFFIX) \
> > +#  $(NULL)
> 
> If this isn't needed let's just remove it.

It is as soon as mtransport and signaling land.  I could ifdef MOZ_WEBRTC_SIGNALING instead (actually more correct anyways - I was just trying to get it working, and knew they'd be landing right after this - but ifdefing them is right regardless).
(Assignee)

Comment 6

6 years ago
(In reply to Randell Jesup [:jesup] from comment #5)
> (In reply to Ted Mielczarek [:ted] from comment #4)
> > Comment on attachment 667789 [details] [diff] [review]
> > cleanup from importing webrtc.org update.  Part is bug 778801 (
> > 
> > Review of attachment 667789 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I'd like to run this by glandium since I wrote some of the makefile changes
> > here.
> > 
> > ::: configure.in
> > @@ +2984,5 @@
> > > +MOZ_CHECK_HEADERS([linux/if_addr.h linux/rtnetlink.h],,,[#include <sys/socket.h>])
> > > +
> > > +dnl Try for MMX support
> > > +dnl NB - later gcc versions require -mmmx for this header to be successfully
> > > +dnl included (or another option which implies it, such as -march=pentium-mmx)
> > 
> > Does this note mean we have to add this compiler option to make this
> > actually work?

Sorry, lost this part of the reply - This was a auto-merge foobar; it's not actually needed anymore and is not currently in alder (or head).  I'll remove it.
Comment on attachment 667789 [details] [diff] [review]
cleanup from importing webrtc.org update.  Part is bug 778801 (

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

::: configure.in
@@ +5249,5 @@
>      MOZ_VP8_ENCODER=1
>      MOZ_VP8_ERROR_CONCEALMENT=1
> +    MOZ_WEBRTC_SIGNALING=
> +    if test "${OS_TARGET}" = "WINNT"; then
> +        MOZ_WEBRTC_IN_LIBXUL=1

I'm tempted to say "just set MOZ_WEBRTC_IN_LIBXUL=1 everywhere".

::: toolkit/library/Makefile.in
@@ +388,5 @@
> +#  $(DEPTH)/media/webrtc/signaling/signaling_sipcc/$(LIB_PREFIX)sipcc.$(LIB_SUFFIX) \
> +#  $(NULL)
> +ifdef MOZ_WEBRTC_IN_LIBXUL
> +include $(topsrcdir)/media/webrtc/shared_libs.mk
> +EXTRA_DSO_LDOPTS += $(WEBRTC_LIBS)

Using SHARED_LIBRARY_LIBS should work too, in which case you don't need to change shared_libs.mk for that.

@@ +393,5 @@
> +ifeq (WINNT,$(OS_TARGET))
> +EXTRA_DSO_LDOPTS += \
> +  -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
> +  $(NULL)
> +OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)

These would be better shared in media/webrtc/shared_libs.mk
Attachment #667789 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #7)
> I'm tempted to say "just set MOZ_WEBRTC_IN_LIBXUL=1 everywhere".

I think I tried that and it broke the signaling C++ unittests, and I didn't want to deal with that headache at the time.
(Assignee)

Comment 9

6 years ago
Created attachment 668004 [details] [diff] [review]
response to reviews for WebRTC merge
(Assignee)

Comment 11

6 years ago
Inbound burned on a case-sensitive rename from iSAC to isac.  Repush with iSAC deleted:

https://hg.mozilla.org/integration/mozilla-inbound/rev/174d772e83f4
https://hg.mozilla.org/integration/mozilla-inbound/rev/7dda68e6ffba
https://hg.mozilla.org/mozilla-central/rev/174d772e83f4
https://hg.mozilla.org/mozilla-central/rev/7dda68e6ffba
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Updated

6 years ago
Depends on: 798814

Updated

6 years ago
No longer depends on: 798814

Comment 13

6 years ago
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1349586690.1349587246.26480.gz#err0

/builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/third_party/libyuv/source/compare.cc: Assembler messages:
/builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/third_party/libyuv/source/compare.cc:220: Error: no such instruction: `pmulld %xmm6,%xmm0'
/builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/third_party/libyuv/source/compare.cc:226: Error: no such instruction: `pmulld %xmm5,%xmm3'
/builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/third_party/libyuv/source/compare.cc:230: Error: no such instruction: `pmulld %xmm5,%xmm4'
/builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/third_party/libyuv/source/compare.cc:235: Error: no such instruction: `pmulld %xmm5,%xmm2'
/builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/third_party/libyuv/source/compare.cc:238: Error: no such instruction: `pmulld %xmm5,%xmm1'
NEXT ERROR make[7]: *** [source/compare.o] Error 1

Updated

6 years ago
Depends on: 798921
(In reply to Philip Chee from comment #13)
> /builds/slave/comm-cen-trunk-lnx64/build/mozilla/media/webrtc/trunk/
> third_party/libyuv/source/compare.cc:220: Error: no such instruction:
> `pmulld %xmm6,%xmm0'

This is an SSE4.1 instruction. What assembler are you using?

Comment 15

6 years ago
> This is an SSE4.1 instruction. What assembler are you using?
See Bug 798921

Updated

6 years ago
Whiteboard: [qa-]

Updated

5 years ago
Depends on: 813989
(Assignee)

Updated

5 years ago
Blocks: 818631

Updated

2 years ago
Depends on: 1285501
You need to log in before you can comment on or make changes to this bug.