The culprits are: logTag in peerconnection and sdp and getLogModule in mediapipeline. Fixing this will allow us to build these files using unified build which should speed things up.

Try job here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eba2c5aec7036829ab6dfb7069ef95a40883d652

Comment on attachment 8914880 [details] Bug 1402334 - Build media-conduit and peerconnection using unified build; https://reviewboard.mozilla.org/r/185800/#review191192 r+, but.... I'd prefer something that used CPP magic to avoid some of this junk static const char pciLogTag "..." #define LOGTAG pciLogTag ... CSFLogDebug(LOGTAG, ....) Even better (though more work) would be to define macros like #define LogDebug(...) CSFLogDebug(LOGTAG, ...) (though that syntax doesn't work, and it may be that defining them to do this is a pain, or requires LOG(("xxxxxx", args)) type of macro)

Comment on attachment 8914881 [details] Bug 1402334 - Add log level test to CSFLog; https://reviewboard.mozilla.org/r/185802/#review191232 ::: media/webrtc/signaling/src/common/browser_logging/CSFLog.h:41 (Diff revision 1) > #endif > ; > > void CSFLogV( CSFLogLevel priority, const char* sourceFile, int sourceLine, const char* tag , const char* format, va_list args); > > +int CSFLogTestLevel(CSFLogLevel priority); Not a big fan of the name. Test made me think of a unit test first. But since it's only exposing the underlying test function I guess it's okay.

Comment on attachment 8914882 [details] Bug 1402334 - Build mediapipeline using unified build; https://reviewboard.mozilla.org/r/185804/#review191242 ::: media/webrtc/signaling/src/mediapipeline/RtpLogger.cpp (Diff revision 1) > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > // Original author: nohlmeier@mozilla.com > > #include "RtpLogger.h" > -#include "logging.h" No '#include "CSFLog.h"' needed here? ::: media/webrtc/signaling/src/mediapipeline/moz.build:20 (Diff revision 1) > '/media/webrtc/trunk', > '/netwerk/srtp/src/crypto/include', > '/netwerk/srtp/src/include', > ] > > # Duplicate definition of getLogModule Does this comment still apply?

Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4eb36d1710ca Build media-conduit and peerconnection using unified build; r=jesup https://hg.mozilla.org/integration/autoland/rev/de7bd38908da Add log level test to CSFLog; r=drno https://hg.mozilla.org/integration/autoland/rev/978b0400ac3b Build mediapipeline using unified build; r=drno

Backed out for bustage at media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:609: 'logTag' was not declared in this scope: https://hg.mozilla.org/integration/autoland/rev/1d9045285aa9a481f4d115153991178a8485fc53 https://hg.mozilla.org/integration/autoland/rev/0435ae6ad52982e974037f74db0d87e1333f8770 https://hg.mozilla.org/integration/autoland/rev/f2040cc62aba75bc1fef1533474c8b957e44e4dd Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=978b0400ac3b1bdbb074c3a759144caec58ba90c&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=134953015&repo=autoland [task 2017-10-04T17:10:23.975Z] 17:10:23 INFO - In file included from /builds/worker/workspace/build/src/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:5:0, [task 2017-10-04T17:10:23.976Z] 17:10:23 INFO - from /builds/worker/workspace/build/src/obj-firefox/media/webrtc/signaling/src/media-conduit/Unified_cpp_src_media-conduit0.cpp:2: [task 2017-10-04T17:10:23.976Z] 17:10:23 INFO - /builds/worker/workspace/build/src/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp: In member function 'virtual mozilla::MediaConduitErrorCode mozilla::WebrtcAudioConduit::EnableMIDExtension(bool, uint8_t)': [task 2017-10-04T17:10:23.976Z] 17:10:23 INFO - /builds/worker/workspace/build/src/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:609:15: error: 'logTag' was not declared in this scope [task 2017-10-04T17:10:23.976Z] 17:10:23 INFO - CSFLogDebug(logTag, "%s %d %d ", __FUNCTION__, enabled, id); [task 2017-10-04T17:10:23.976Z] 17:10:23 INFO - ^ [task 2017-10-04T17:10:23.976Z] 17:10:23 INFO - /builds/worker/workspace/build/src/media/webrtc/signaling/src/common/browser_logging/CSFLog.h:24:84: note: in definition of macro 'CSFLogDebug' [task 2017-10-04T17:10:23.977Z] 17:10:23 INFO - #define CSFLogDebug(tag , format, ...) CSFLog(CSF_LOG_DEBUG, __FILE__ , __LINE__ , tag , format , ## __VA_ARGS__ ) [task 2017-10-04T17:10:23.977Z] 17:10:23 INFO - ^~~ [task 2017-10-04T17:10:23.977Z] 17:10:23 INFO - /builds/worker/workspace/build/src/config/rules.mk:1075: recipe for target 'Unified_cpp_src_media-conduit0.o' failed [task 2017-10-04T17:10:23.977Z] 17:10:23 INFO - gmake[5]: *** [Unified_cpp_src_media-conduit0.o] Error 1

Sorry, looks like another use of logTag got added while I was working on this. I'll do a manual rebase and build on top of inbound and try again.

To be safe, I'll wait until tomorrow morning after the autoland repo has been merged back to inbound, as the new use of logTag only seems to be there.

Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/807ccf63a12f Build media-conduit and peerconnection using unified build; r=jesup https://hg.mozilla.org/integration/mozilla-inbound/rev/0712e0bd9e12 Add log level test to CSFLog; r=drno https://hg.mozilla.org/integration/mozilla-inbound/rev/808190cb16d0 Build mediapipeline using unified build; r=drno

https://hg.mozilla.org/mozilla-central/rev/807ccf63a12f https://hg.mozilla.org/mozilla-central/rev/0712e0bd9e12 https://hg.mozilla.org/mozilla-central/rev/808190cb16d0

Hi Dan I see the build error in my macbook with macos 10.11.6. Should I check my building environment? 0:23.80 In file included from /Users/seanlee/Projects/mozilla/src/gecko/obj-x86_64-apple-darwin15.6.0/media/webrtc/signaling/src/media-conduit/Unified_cpp_src_media-conduit0.cpp:29: 0:23.80 /Users/seanlee/Projects/mozilla/src/gecko/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:34:10: fatal error: 'webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h' file not found 0:23.80 #include "webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h" 0:23.80 ^ (In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #16) > https://hg.mozilla.org/mozilla-central/rev/807ccf63a12f I check out to the above commit's parent[1], and the error is gone. [1] https://hg.mozilla.org/mozilla-central/rev/eab45e291bc0

I'm guessing that the switch to unified build has somehow broken this #ifdef: #if defined(MAC_OS_X_VERSION_10_8) && \ (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_8) // XXX not available in Mac 10.7 SDK #include "webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h" #endif I'm not sure why that would happen, I'll try to duplicate on my macbook. Sorry for the inconvenience.

(In reply to Dan Minor [:dminor] from comment #18) > I'm guessing that the switch to unified build has somehow broken this #ifdef: > #if defined(MAC_OS_X_VERSION_10_8) && \ > (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_8) > // XXX not available in Mac 10.7 SDK > #include "webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h" > #endif > > I'm not sure why that would happen, I'll try to duplicate on my macbook. > Sorry for the inconvenience. Note that local builds may be different that automation builds, depending on what SDK you have installed. We *just* switched automation to 10.11 SDK I believe from 10.7 (in the last few weeks).

Comment on attachment 8918834 [details] [diff] [review] Fix static analysis warning Review of attachment 8918834 [details] [diff] [review]: ----------------------------------------------------------------- Concern about the perf impact (at a micro level) ::: media/webrtc/signaling/src/common/browser_logging/CSFLog.cpp @@ +42,5 @@ > printf("%s\n:",tag); > vprintf(format, args); > #else > > + mozilla::LogLevel level = CSFLogLevelToMozLogLevel(priority); Wouldn't = static_cast<mozilla::LogLevel>(static_cast<unsigned int>(priority)) bypass this without introducing a function? Admittedly a hopefully-inlined function you can hope the compiler makes into an array access -- but it will still have to add branches to *every* use of CSFLog. Or better, get rid of the separate logging levels and typedef CSFLogLevel to mozilla::LogLevel, and #define the CSF_LOG_* entries to LogLevel entries. This would make it dependent on LogLevel/Logging.h --- but it is already at the cpp level, this would just add a dependency at the .h level.

(In reply to Randell Jesup [:jesup] from comment #21) > Comment on attachment 8918834 [details] [diff] [review] > Fix static analysis warning > > Review of attachment 8918834 [details] [diff] [review]: > ----------------------------------------------------------------- > > Concern about the perf impact (at a micro level) > > ::: media/webrtc/signaling/src/common/browser_logging/CSFLog.cpp > @@ +42,5 @@ > > printf("%s\n:",tag); > > vprintf(format, args); > > #else > > > > + mozilla::LogLevel level = CSFLogLevelToMozLogLevel(priority); > > Wouldn't = static_cast<mozilla::LogLevel>(static_cast<unsigned > int>(priority)) bypass this without introducing a function? Admittedly a > hopefully-inlined function you can hope the compiler makes into an array > access -- but it will still have to add branches to *every* use of CSFLog. > > Or better, get rid of the separate logging levels and typedef CSFLogLevel to > mozilla::LogLevel, and #define the CSF_LOG_* entries to LogLevel entries. > This would make it dependent on LogLevel/Logging.h --- but it is already at > the cpp level, this would just add a dependency at the .h level. We include this header from C code, so unfortunately typedefing CSFLogLevel to mozilla::LogLevel won't work. I can try the double cast and see if that is enough to fool the static analysis.

(In reply to Sean Lee [:seanlee][:weilonge] from comment #17) > Hi Dan > > I see the build error in my macbook with macos 10.11.6. Should I check my > building environment? > > 0:23.80 In file included from > /Users/seanlee/Projects/mozilla/src/gecko/obj-x86_64-apple-darwin15.6.0/ > media/webrtc/signaling/src/media-conduit/Unified_cpp_src_media-conduit0.cpp: > 29: > 0:23.80 > /Users/seanlee/Projects/mozilla/src/gecko/media/webrtc/signaling/src/media- > conduit/VideoConduit.cpp:34:10: fatal error: > 'webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h' file not found > 0:23.80 #include > "webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h" > 0:23.80 ^ > > (In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on > intermittent or backout) from comment #16) > > https://hg.mozilla.org/mozilla-central/rev/807ccf63a12f > I check out to the above commit's parent[1], and the error is gone. > > [1] https://hg.mozilla.org/mozilla-central/rev/eab45e291bc0 Hi Sean, I'm not able to reproduce this problem on my macbook, but I had to go through a large build environment update prior to being able to build. As Randell said, we now require the 10.11 SDK to build in automation, which is probably the update I had to apply on my machine. Maybe try updating your build environment and see if the problem persists? Sorry, Dan

I cannot build after landing this too when I use 10.11 SDK. Maybe, before landing this, comment #18 condition isn't built. And if building it, #include "webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h" is always error. (correct path is webrtc/common_video/include/corevideo_frame_buffer.h)

And VideoFrame class has no nativeHandle member. Maybe, it is frame.video_frame_bufer()->native_handle() instead.

3 issues - include path (webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h) is invalid. - webrtc namespace is missing - VideoFrame class has no nativeHandle member. I think that this condition's code isn't built unfortunately until using unified build.

If we use MAC_OS_X_VERSION_10_8, it requires AvailabilityMacros.h.

(In reply to Makoto Kato [:m_kato] from comment #27) > Created attachment 8919175 [details] [diff] [review] > Fix bustage > > 3 issues > - include path (webrtc/sdk/objc/Framework/Classes/corevideo_frame_buffer.h) > is invalid. > - webrtc namespace is missing > - VideoFrame class has no nativeHandle member. > > I think that this condition's code isn't built unfortunately until using > unified build. Thank you for putting together a patch for this.

Comment on attachment 8919183 [details] [diff] [review] Fix bustage v2 Review of attachment 8919183 [details] [diff] [review]: ----------------------------------------------------------------- r+ with needed ifdefs ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp @@ +28,5 @@ > #include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" > #include "webrtc/common_video/include/video_frame_buffer.h" > #include "webrtc/api/video/i420_buffer.h" > + > +#include <AvailabilityMacros.h> Is this really available on Windows and Linux/etc? I think this needs an ifdef XP_MACOSX or some such

(In reply to Randell Jesup [:jesup] from comment #31) > Comment on attachment 8919183 [details] [diff] [review] > > Is this really available on Windows and Linux/etc? I think this needs an > ifdef XP_MACOSX or some such Thanks, I forget it for non-Mac. I add #ifdef WEBRTC_MAC

Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/mozilla-inbound/rev/f605961878cc Fix bustage for 10.11 SDK. r=jesup

Comment on attachment 8918940 [details] [diff] [review] Fix static analysis warning Review of attachment 8918940 [details] [diff] [review]: ----------------------------------------------------------------- Not pretty, but should do the job.

Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8f7077cf15e4 Fix static analysis warning in CSFLog.cpp; r=drno