Closed Bug 1166955 Opened 5 years ago Closed 3 years ago

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

Categories

(Core :: WebRTC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox41 --- affected
firefox55 --- fixed
Blocking Flags:
backlog tech-debt

People

(Reporter: dholbert, Assigned: emk)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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).
You'll need to ask jesup about changes to media/webrtc/trunk/webrtc/modules/video_capture/windows/BaseFilter.cpp, it's webrtc code.
Flags: needinfo?(rjesup)
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
Flags: needinfo?(rjesup)
Component: Audio/Video → WebRTC
backlog: --- → tech-debt
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
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
Attachment #8874089 - Flags: review?(rjesup) → review+
Blocks: 1370054
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/ae2ff08a40b0
Stop including nsAutoPtr.h from BasePin.cpp. r=jesup
https://hg.mozilla.org/mozilla-central/rev/ae2ff08a40b0
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.