Closed Bug 797671 Opened 7 years ago Closed 7 years ago

Update webrtc.org code from stable branch 3.12

Categories

(Core :: WebRTC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jesup, Assigned: jesup)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(3 files)

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
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)
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+
(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).
(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.
https://hg.mozilla.org/mozilla-central/rev/174d772e83f4
https://hg.mozilla.org/mozilla-central/rev/7dda68e6ffba
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Depends on: 798814
No longer depends on: 798814
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
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?
> This is an SSE4.1 instruction. What assembler are you using?
See Bug 798921
Whiteboard: [qa-]
Depends on: 813989
Depends on: 1285501
You need to log in before you can comment on or make changes to this bug.