823921 - Fix WMF backend compilation on mingw

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jacek Caban]

Attachment: linkage fix

The attached patch uses linker options for specifying linked libraries.

It also undefined GetCurrentTime in MediaDecoder.h. That part is not really mingw-dependent. It's a problem with a macro in windows headers and it's an accident that mingw trigers it. MSVC could just as well, depending on headers include orider. We already do this in a few places across the tree.

Comment 1

[Jacek Caban]

Attachment: Use RefPtr instead of _com_ptr_t

mingw has limited support for _com_ptr_t (aka. _COM_SMARTPTR_TYPEDEF). I started looking at fixing it there, but realized that it would be better (also for Mozilla) to fix it here. _com_ptr_t handles errors using exceptions, which are not used in Mozilla code. The attached patch replaces usage of _com_ptr_t by RefPtr, like most other code that deals with MS COM.

Comment 2

[Jacek Caban]

Attachment: warning fixes

Warning fixes. As separate patch for easier review and separation of less important part.

Comment 3

[Jacek Caban]

Comment on attachment 694835 [details] [diff] [review]
Use RefPtr instead of _com_ptr_t

I forgot to mention, this is green on try: https://tbpl.mozilla.org/?tree=Try&rev=b53b4aa772a5

Comment 4

[Paul Adenot (:padenot)]

Comment on attachment 694836 [details] [diff] [review]
warning fixes

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

Thanks!

Comment 5

[Jacek Caban]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9059edd199b2

Comment 6

[Paul Adenot (:padenot)]

Comment on attachment 694835 [details] [diff] [review]
Use RefPtr instead of _com_ptr_t

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

Yeah, this aligns better with other Windows specific code we have in the tree (mainly D2D draw target, from what I read).

Thanks.

::: content/media/wmf/WMFByteStream.cpp
@@ +180,2 @@
>    NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
> +  hr = unknown->QueryInterface((IMFAsyncResult**)byRef(callerResult));

Can you |static_cast<IMFAsyncResult**>(byRef(callerResult))| here instead?

::: content/media/wmf/WMFReader.cpp
@@ +512,5 @@
>    // but only some systems (Windows 8?) support it.
>    BYTE* data = nullptr;
>    LONG stride = 0;
> +  RefPtr<IMF2DBuffer> twoDBuffer;
> +  hr = buffer->QueryInterface((IMF2DBuffer**)byRef(twoDBuffer));

Same thing, prefer C++ style casts. static_cast<> also works in this case.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

https://hg.mozilla.org/mozilla-central/rev/9059edd199b2

Comment 8

[Jacek Caban]

Thanks for the review, pushed a version with static_casts:

https://hg.mozilla.org/integration/mozilla-inbound/rev/158a9c8668bb

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/158a9c8668bb

Comment 10

[Jacek Caban]

https://hg.mozilla.org/integration/mozilla-inbound/rev/21d5fbda37b8

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/21d5fbda37b8

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Jacek Caban from comment #3)
> Comment on attachment 694835 [details] [diff] [review]
> Use RefPtr instead of _com_ptr_t
> 
> I forgot to mention, this is green on try:
> https://tbpl.mozilla.org/?tree=Try&rev=b53b4aa772a5

Note that the WMF backend is currently disabled on m-c by default, so unless you toggle the pref in modules/libpref/src/init/all.js, WMF won't actually be tested... And currently the tests still randomly fail for WMF anyway...

BTW, we're still using _com_ptr_t in webrtc:
http://mxr.mozilla.org/mozilla-central/search?string=_COM_SMARTPTR_TYPEDEF
(I wrote at least some of that code too, sorry!)