msdmo should be delay loaded dll w/ MOZ_WEBRTC_IN_LIBXUL

RESOLVED FIXED in mozilla22

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla22
All
Windows 8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [webrtc][blocking-webrtc+][qa-])

Attachments

(1 attachment)

Created attachment 716970 [details] [diff] [review]
fix

By bug 780069, msdmo.dll is delay loading dll.  But it isn't be merged with MOZ_WEBRTC_IN_LIBXUL.
Attachment #716970 - Flags: review?(rjesup)
Comment on attachment 716970 [details] [diff] [review]
fix

-> ted as this is a build patch

I notice a bunch of libs were removed - these were copied in from the gyp/etc stuff; they may not actually be needed - ted, opinions?

If this is r+'d, please push a windows Try (win64 too perhaps) with tests before landing.
Attachment #716970 - Flags: review?(rjesup) → review?(ted)
blocking not decided yet.  

What practical impact does this have?  Faster FF load times?  If so, how much?  And if so and "large enough", since this code is already in production builds there's a question if uplift to Aurora or Beta would be in order.
Whiteboard: [webrtc][blocking-webrtc?]
Comment on attachment 716970 [details] [diff] [review]
fix

Review of attachment 716970 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/media/Makefile.in
@@ -152,5 @@
> -ifdef MOZ_WEBRTC
> -EXTRA_DSO_LDOPTS += \
> -  -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
> -  $(NULL)
> -OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)

I assume that if we actually needed any of this we'd get link errors with this removed.
Attachment #716970 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> Comment on attachment 716970 [details] [diff] [review]
> fix
> 
> Review of attachment 716970 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/media/Makefile.in
> @@ -152,5 @@
> > -ifdef MOZ_WEBRTC
> > -EXTRA_DSO_LDOPTS += \
> > -  -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
> > -  $(NULL)
> > -OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
> 
> I assume that if we actually needed any of this we'd get link errors with
> this removed.

this is duplicated lines.  

http://mxr.mozilla.org/mozilla-central/source/layout/media/Makefile.in?mark=136-156#136

136 ifdef MOZ_WEBRTC
137 EXTRA_DSO_LDOPTS += \
138   -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
139   $(NULL)
140 OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
141 ifdef _MSC_VER
142 OS_LIBS += $(call EXPAND_LIBNAME,delayimp)
143 EXTRA_DSO_LDOPTS += \
144   -DELAYLOAD:msdmo.dll \
145   $(NULL)
146 endif
147 endif
148 
149 ifdef MOZ_WEBRTC
150 EXTRA_DSO_LDOPTS += \
151   -LIBPATH:"$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)" \
152   $(NULL)
153 OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
154 endif
155 DEFFILE = symbols.def
156 endif
Whiteboard: [webrtc][blocking-webrtc?] → [webrtc][blocking-webrtc+]
Is this patch ready to check in?  If so, mark the patch checkin-needed (you can target it to me)
Flags: needinfo?(m_kato)
Ah, I'm sorry.  I will land this soon.
Flags: needinfo?(m_kato)
https://hg.mozilla.org/mozilla-central/rev/fcc9af890fed
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [webrtc][blocking-webrtc+] → [webrtc][blocking-webrtc+][qa-]
You need to log in before you can comment on or make changes to this bug.