Closed Bug 1176730 Opened 4 years ago Closed 4 years ago

Don't use pthread for libvpx in mingw builds.

Categories

(Core :: Audio/Video, defect)

Unspecified
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- unaffected
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: jacek, Assigned: jacek)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
mingw can optionally have pthread support, but even if it does, it doesn't make sense to use it on Windows when support for native Windows threading is provided anyway. Currently, mingw build without pthread fails.
Attachment #8625317 - Flags: review?(j)
Comment on attachment 8625317 [details] [diff] [review]
patch

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

Patch looks ok but needs to be added to update.py so its not lost with the next update.
Attachment #8625317 - Flags: review?(j) → review-
Attachment #8625317 - Attachment is obsolete: true
Attachment #8625425 - Flags: review?(giles)
Comment on attachment 8625425 [details] [diff] [review]
0001-Bug-1176730-Don-t-use-pthread-for-libvpx-in-mingw-bu.patch

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

::: media/libvpx/vpx_config_x86-win32-gcc.h
@@ +35,4 @@
>  #define HAVE_VPX_PORTS 1
>  #define HAVE_STDINT_H 1
>  #define HAVE_ALT_TREE_LAYOUT 0
> +#undef HAVE_PTHREAD_H

Why is the undef necessary?
(In reply to Ralph Giles (:rillian) from comment #3)
> Comment on attachment 8625425 [details] [diff] [review]
> 0001-Bug-1176730-Don-t-use-pthread-for-libvpx-in-mingw-bu.patch
> 
> Review of attachment 8625425 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/libvpx/vpx_config_x86-win32-gcc.h
> @@ +35,4 @@
> >  #define HAVE_VPX_PORTS 1
> >  #define HAVE_STDINT_H 1
> >  #define HAVE_ALT_TREE_LAYOUT 0
> > +#undef HAVE_PTHREAD_H
> 
> Why is the undef necessary?

We have a check for pthread.h in configure script. If it's found then HAVE_PTHREAD_H is defined in mozilla-config.h and we have a warning here.
Comment on attachment 8625425 [details] [diff] [review]
0001-Bug-1176730-Don-t-use-pthread-for-libvpx-in-mingw-bu.patch

Ok, looks fine to me.
Attachment #8625425 - Flags: review?(giles) → review+
https://hg.mozilla.org/mozilla-central/rev/30f68d9c65a5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8625425 [details] [diff] [review]
0001-Bug-1176730-Don-t-use-pthread-for-libvpx-in-mingw-bu.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1151175
[User impact if declined]: This is needed for uplift of newer libvpx changes.
[Describe test coverage new/current, TreeHerder]: Landed on m-c some time ago.
[Risks and why]: Risk is minimal. This doesn't affect officially supported builds.
[String/UUID change made/needed]: None.
Attachment #8625425 - Flags: approval-mozilla-beta?
Blocks: 1178215
Flags: needinfo?(abillings)
Comment on attachment 8625425 [details] [diff] [review]
0001-Bug-1176730-Don-t-use-pthread-for-libvpx-in-mingw-bu.patch

As Ralph stated, this is a no-op for officially supported builds. Beta+
Attachment #8625425 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Don't we need this on Aurora as well?
Flags: needinfo?(abillings)
https://hg.mozilla.org/releases/mozilla-beta/rev/b92f62aa6643

(In reply to Al Billings [:abillings] from comment #10)
> Don't we need this on Aurora as well?

This landed prior to the uplift.
You need to log in before you can comment on or make changes to this bug.