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

RESOLVED FIXED in Firefox 55

Status

()

defect
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: dholbert, Assigned: emk)

Tracking

(Blocks 1 bug)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 affected, firefox55 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
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
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED

Comment 4

2 years ago
mozreview-review
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+
Assignee

Updated

2 years ago
Blocks: 1370054

Comment 5

2 years ago
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: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.