1166955 - nsISupportsUtils.h(49) : warning C4005: 'NS_IF_ADDREF' : macro redefinition, for Windows WebRTC file "BaseFilter.cpp" (maybe partly due to unified builds)

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Daniel Holbert [:dholbert]]

Noticed these build warnings go by in a windows build:
{
Unified_cpp_webrtc_modules0.cpp
Warning: C4005 in $OBJDIR\dist\include\nsISupportsUtils.h: 'NS_IF_ADDREF' : macro redefinition
$OBJ\dist\include\nsISupportsUtils.h(49) : warning C4005: 'NS_IF_ADDREF' : macro redefinition
        $SRC/media/webrtc/trunk/webrtc/modules/video_capture/windows/BaseFilter.cpp(17) : see previous definition of 'NS_IF_ADDREF'
Warning: C4005 in $OBJDIR\dist\include\nsISupportsUtils.h: 'NS_IF_RELEASE' : macro redefinition
$OBJ\dist\include\nsISupportsUtils.h(101) : warning C4005: 'NS_IF_RELEASE' : macro redefinition
        $SRC/media/webrtc/trunk/webrtc/modules/video_capture/windows/BaseFilter.cpp(22) : see previous definition of 'NS_IF_RELEASE'
video_capture_module_internal_impl.lib.desc
rm -f video_capture_module_internal_impl.lib
}

Looks like we have hardcoded NS_IF_ADDREF / RELEASE macros in BaseFilter.cpp.

cpearce, can we just explicitly include nsISupportsUtils.h here? (to get the macros from that file)

I think we're already implicitly getting nsISupportsUtils.h's versions via this .cpp file being unified with other .cpp files (and then this file nds up redefining macros that it doesn't expect to be defined).

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

You'll need to ask jesup about changes to media/webrtc/trunk/webrtc/modules/video_capture/windows/BaseFilter.cpp, it's webrtc code.

Comment 2

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

Those files (the replacements for MS example code in media/webrtc/trunk/webrtc/modules/video_capture/windows) don't include any mozilla/xpcom stuff at all.  About the only mozilla-ism in there at all is "namespace mozilla {\n namespace media {".   We shouldn't add mozilla/xpcom includes.

Thus I think it would be better to have them not get merged with stuff which includes mozilla things.  I'd even be fine with moving Base*/DShowTools.h/MediaType* into a sub-dir if that made it easier.  (The directory has these and a bunch of files that are part of the webrtc.org distribution.

In fact, a much better location (but probably more makefile work) would be media/webrtc/windows_base or media/webrtc/trunk/windows_base

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: Bug 1166955 - Stop including nsAutoPtr.h from BasePin.cpp.

Review commit: https://reviewboard.mozilla.org/r/145552/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/145552/

Comment 4

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

Comment on attachment 8874089 [details]
Bug 1166955 - Stop including nsAutoPtr.h from BasePin.cpp.

https://reviewboard.mozilla.org/r/145552/#review149510

Please file a followup bug to move the files here to media/webrtc/windows_base

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae2ff08a40b0
Stop including nsAutoPtr.h from BasePin.cpp. r=jesup

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ae2ff08a40b0