1176730 - Don't use pthread for libvpx in mingw builds.

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jacek Caban]

Attachment: patch

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.

Comment 1

[Jan Gerber]

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.

Comment 2

[Jan Gerber]

Attachment: 0001-Bug-1176730-Don-t-use-pthread-for-libvpx-in-mingw-bu.patch

Comment 3

[Ralph Giles (:rillian)]

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?

Comment 4

[Jacek Caban]

(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 5

[Ralph Giles (:rillian)]

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.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30f68d9c65a5

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/30f68d9c65a5

Comment 8

[Ralph Giles (:rillian)]

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.

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

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+

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Don't we need this on Aurora as well?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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.