mingw build broken with VideoDecoder.cpp error: 'std::thread' has not been declared

RESOLVED FIXED in Firefox 55

Status

()

Core
Audio/Video: GMP
P2
normal
Rank:
25
RESOLVED FIXED
9 months ago
3 months ago

People

(Reporter: tjr, Assigned: tjr)

Tracking

(Blocks: 1 bug)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [tor])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
mingw does not understand std::thread:

> 0:13.12 In file included from /tmp/workspace/mingw-w64-build/mozilla-mingw/media/gmp-clearkey/0.1/Unified_cpp_gmp-clearkey_0.10.cpp:74:0:
> 0:13.12 /tmp/workspace/mingw-w64-build/firefox/media/gmp-clearkey/0.1/VideoDecoder.cpp: In constructor 'VideoDecoder::VideoDecoder(cdm::Host_8*)':
> 0:13.12 /tmp/workspace/mingw-w64-build/firefox/media/gmp-clearkey/0.1/VideoDecoder.cpp:40:38: error: 'std::thread' has not been declared
> 0:13.12    uint32_t cores = std::max(1u, std::thread::hardware_concurrency());

In a related bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1317642) we commented out the code on MinGW because it was in Error Reporting and not strictly necessary. Here we could use something like the below, or just hardcode it.

> #ifdef _WIN32
>     SYSTEM_INFO sysinfo;
>     GetSystemInfo(&sysinfo);
>     return sysinfo.dwNumberOfProcessors;

Updated

9 months ago
Rank: 25
Priority: -- → P2
(Assignee)

Updated

9 months ago
Blocks: 1349912
(Assignee)

Updated

9 months ago
Assignee: nobody → tom
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Attachment #8850640 - Flags: review?(cpearce)

Comment 2

9 months ago
mozreview-review
Comment on attachment 8850640 [details]
Bug 1344909 When compiled with MinGW, use only a single core in gmp-clearkey

https://reviewboard.mozilla.org/r/123184/#review127122

Thanks for the patch. It's a shame mingw doesn't have std::thread yet. I'd be happy to for us to get the number of processors from a call to GetSystemInfo() as well, as it would mean mingw builds would have better parallelism for ClearKey video. There's very little ClearKey content out there through, so it's not super important.
Attachment #8850640 - Flags: review?(cpearce) → review+
(Assignee)

Comment 3

9 months ago
(In reply to Chris Pearce (:cpearce) from comment #2)
> Comment on attachment 8850640 [details]
> Bug 1344909 When compiled with MinGW, use only a single core in gmp-clearkey
> 
> https://reviewboard.mozilla.org/r/123184/#review127122
> 
> Thanks for the patch. It's a shame mingw doesn't have std::thread yet. I'd
> be happy to for us to get the number of processors from a call to
> GetSystemInfo() as well, as it would mean mingw builds would have better
> parallelism for ClearKey video. There's very little ClearKey content out
> there through, so it's not super important.

Thanks! The plan is to add pthreads in mingw to get std::thread support at which point I'll be undoing the few patches like these we've accumulated, so the GetSystemInfo() approach isn't really worth the complexity of having that code in FF without it being exercised by our tests right now.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/142baaabe917
When compiled with MinGW, use only a single core in gmp-clearkey r=cpearce
Keywords: checkin-needed

Comment 5

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/142baaabe917
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Guessing this is wontfix for 54 as tor builds aren't based off that version.
status-firefox54: affected → wontfix
(Assignee)

Updated

3 months ago
See Also: → bug 1395418
You need to log in before you can comment on or make changes to this bug.