Closed Bug 1275541 Opened 5 years ago Closed 5 years ago
Fix the name conflict of macro Get
Current Time() in Win Bas .h and Media Stream::Get Current Time()
58 bytes, text/x-review-board-request
Follow bug 1235301 comment 73. In the try of bug 1235301, Window does not compile which is caused by the name conflict of macro GetCurrentTime() in WinBas.h and MediaStream::GetCurrentTime(). The failed try job is: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fa6727ad16c&selectedJob=20335318
Review commit: https://reviewboard.mozilla.org/r/55078/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55078/
Attachment #8756298 - Flags: review?(rjesup)
Comment on attachment 8756298 [details] MozReview Request: Bug 1275541 - undefine GetCurrentTime; r?jesup https://reviewboard.mozilla.org/r/55078/#review51732 Stealing the review, this is trivial and we've got the same fix in other locations of the code.
Attachment #8756298 - Flags: review+
Attachment #8756298 - Flags: review?(rjesup) → review+
Comment on attachment 8756298 [details] MozReview Request: Bug 1275541 - undefine GetCurrentTime; r?jesup https://reviewboard.mozilla.org/r/55078/#review51734
Component: Audio/Video → Audio/Video: MediaStreamGraph
Priority: -- → P1
Try1: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4800b1d6e4ed Try2: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c55de20682d I don't know why tests do not run in Win8 in the first try so I have a second try and all look good. Thanks for the reviews!
Personally I think putting the "#ifndef GetCurrentTime #define GetCurrentTime #endif" everywhere is somewhat annoying and it's not even obvious where to put this period of code since it depends on the include order. Does bug 834505 a good solution to get rid of all the fix? or how about we just rename the MediaStream::GetCurrentTime()?
You need to log in before you can comment on or make changes to this bug.