Closed Bug 1033122 Opened 8 years ago Closed 8 years ago
be consistent about speex
_resampler symbol visibility
Currently the presence of speex/speex_resampler.h in dist/system_wrappers means that some object files are looking for a hidden symbol and others a default symbol. This is because the header is included with "" and so another exported header (in dist/include) finds the speex/speex_resampler.h in dist/include before dist/system_wrappers. Visibility depends on the order of includes. (Fortunately a hidden definition seems to resolve a default visibility undefined symbol.) The convention is to use "" for includes of exported headers within mozilla. <> would probably work but convention implies this is then a system header. We don't want the system header, which may be different from the internal header. If GKMEDIAS_SHARED_LIBRARY is ever meant to work for non-WINNT platforms, then I guess the approach should be to change to <speex/speex_resampler.h> everywhere and wrap only when GKMEDIAS_SHARED_LIBRARY is set. Its only necessary to use <> in exported headers, but code gets copied, so consistency is preferred. I don't know of any intention to support GKMEDIAS_SHARED_LIBRARY elsewhere. A similar approach is required if system includes are ever to be wrapped on WINNT (c.f. bug 1008192). If the GKMEDIAS_SHARED_LIBRARY is only ever WINNT and WRAP_SYSTEM_INCLUDES will never be WINNT, then we could follow the "speex/speex_resampler.h" convention everywhere.
Comment on attachment 8449119 [details] [diff] [review] patch The trunk/webrtc include is already a modification from trunk. https://hg.mozilla.org/mozilla-central/rev/ad8a86bfd860#l9.12
Attachment #8449119 - Flags: review?(khuey) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.