Closed Bug 1101037 Opened 5 years ago Closed 5 years ago

Fix dom/media/gmp compilation with mingw.

Categories

(Core :: Audio/Video, defect)

All
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(1 file)

Attached patch fixSplinter Review
For scope_ptr.h the error is:

scoped_ptr.h:21:20: error: ‘operator=’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
      operator=(other);
                     ^
scoped_ptr.h:21:20: note: declarations in dependent base ‘scoped_array<wchar_t>’ are not found by unqualified lookup
scoped_ptr.h:21:20: note: use ‘this->operator=’ instead

Also GCC doesn't support unicode stream::open. UTF8 is also not perfect, but works... Also system includes should be lowercased for cross compilation.
Attachment #8524706 - Flags: review?(cpearce)
Comment on attachment 8524706 [details] [diff] [review]
fix

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

::: dom/media/gmp/GMPChild.cpp
@@ +315,3 @@
>    stream.open(path.get());
> +#else
> +  stream.open(NS_ConvertUTF16toUTF8(path).get());

Would just always doing this NS_ConvertUTF16toUTF8 Just Work? It's working for me here with MSVC. i.e. can we remove the above #ifdef _MSC_VER block?

r+ either way.
Attachment #8524706 - Flags: review?(cpearce) → review+
Technically, using UTF8 here is wrong. A better thing to do would be converting to system locale. That, however, brings another problem - Windows stores file names in UTF16, which may not be able to encode in current system locale. That's why using UTF16 here is better.

Since we don't have an option of using UTF16 on mingw in this case, I changed it to UTF8 just because we have a nice helper for that. I wanted to keep it simple to not complicate the code non-tier1 platform corner cases.
OK. Best to leave it as is then. Thanks!
https://hg.mozilla.org/mozilla-central/rev/62af62f4f0a4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.