571165 - Allow libvpx to build on OS/2

Status: RESOLVED WONTFIX

(Core :: Audio/Video: Playback, defect)

Description

[Dave Yeo]

Attachment: First go at building libvpx without assembler. Not for review.

Currently libvpx fails to build on OS/2, even without assembler due to problems such as no pthread semaphores.
Also we should support the x86 assembler optimizations.

Comment 1

[Rich Walsh]

(In reply to comment #0)
> Currently libvpx fails to build on OS/2, even without assembler due to
> problems such as no pthread semaphores.

In the now-defunct liboggplay, I had included a full implementation of posix semaphores.  I still have the code & will see if it can be used as-is or adapted as needed.  (Way back when, it was /media/liboggplay/src/liboggplay/os2_semaphore.c & *.h)

Comment 2

[Dave Yeo]

Attachment: jwasm support, not for review

This is one possible solution, using JWasm (http://www.japheth.de/JWasm.html, binary on Hobbes) to assemble the masm code. Besides the main problem that masm support will probably go away to be replaced by yasm, JWasm has problems with the SSE2 code so I had to disable SSE2 support. Performance is still much better then the C code.
This patch is meant to be applied on top of the previous patch.

Comment 3

[Rich Walsh]

How did you get around the pthread semaphore problem?

Comment 4

[Dave Yeo]

I've just been building with threading disabled. Just needed this,
+#if CONFIG_MULTITHREAD
 #include <semaphore.h>
+#endif

I need to set up a virtual machine to run git in to generate patches for upstream and send this for review as well as more OS/2 support patches.

Comment 5

[Walter Meinl]

(In reply to comment #3)
> How did you get around the pthread semaphore problem?

I could build using Yuri's pthread lib. All about threading is defined in libvpx/vp8/common/threading.h In fact this is an pthread emulation layer for windows (and partly for mac) where macros are used to convert between the pthread functions and respective functions in windows. This macro based emulation is based on a file created by John Hart http://world.std.com/~jmhart/opensource.htm
(you have to go to the end of the site). Maybe we could achieve a similar simple port using the respective OS/2 functions. However, I couldn't find a hint whether OS/2 supports natively thread local storage. In Yuri's pthread package there is a tls.h header file.
It's the question, if we should invest the time and add some OS/2 lines to the threading.h header file and create a simple pthread emulation for OS/2 as well or if we should rely on Yuri's emulation.

Comment 6

[Dave Yeo]

Seems that quite a bit has changed since I first tested building libvpx and had the semaphore related failure (it was the very first public release). For now it would probably be simpler to use Yuri's port, especially if libvpx is using thread local storage. From testing h264 decoding using FFmpeg Yuri's port isn't the fastest but is plenty good enough for now. Eventually we'll need someone with a true multi-core cpu to test.
For now I think the more important thing is to get all the assembly assembling. I've been distracted and haven't worked on this since posting the jwasm patch.

Comment 7

[Rich Walsh]

(In reply to comment #6)
> For now it would probably be simpler to use Yuri's port, especially if
> libvpx is using thread local storage.

Creating an original solution shouldn't be hard at all.  libc06 has a nice emulation of Windows' TLS functions and also offers an inline interlocked-compare-and-exchange function needed to create spinlocks.  Combined with my existing semaphore code, we should be good to go.

> For now I think the more important thing is to get all the assembly
> assembling.

Agreed.  Now that I've downloaded your jwasm port, I'll turn my attention to the sem issue (to the extent my own distractions permit).

Comment 8

[Rich Walsh]

FYI...  Porting the pthread & TLS stuff was trivial (the TLS macros may be there but they're never used).  The only "new" code was my recycled posix-semaphore emulation.  Whether it all works is another story - I'll know in the morning when my build completes.

Comment 9

[Walter Meinl]

(In reply to comment #8)
> FYI...  Porting the pthread & TLS stuff was trivial
That's what I thought, at least for you. For me it took me a week with a lot of searching around, but I managed to get something going by "translating" the windows macros to OS/2. Except with _beginthread where I got warnings about pointers I could compile it and vpx worked)
> (the TLS macros may be there but they're never used).
Agreed, I remmed it out and it didn't hurt.
> The only "new" code was my recycled
> posix-semaphore emulation.  Whether it all works is another story - I'll know
> in the morning when my build completes.
I'm curious about your patch. I'm sure your solution will work.

Comment 10

[Rich Walsh]

Attachment: libvpx - basic OS/2 support - v2

This is an update of Dave's "First go at building libvpx" patch.  Multithreading is enabled and some errata has been corrected or removed.

Comment 11

[Rich Walsh]

Attachment: libvpx - jwasm support - v2

This is an update of Dave's "jwasm support" patch.  Some errata has been corrected or removed.

Comment 12

[Rich Walsh]

Attachment: libvpx - multithreading support

This implements the pthread & TLS macros needed for multithreading and adds os2_semaphore.* to emulate posix semaphores.  It also adds a commandline define to makefile.in to eliminate a conflict between OS/2's typedef for 'BOOL' and libvpx's.

Comment 13

[Rich Walsh]

Dave, looking at your settings for jwasm I see you've set it for a P4.  I don't know if it really makes any difference, but isn't that a bit aggressive for our userbase?  Perhaps P2 would be more appropriate.

Also, I see that there's a setting for the coprocessor which defaults to an 8087.  Would changing that to an 80387 help anyone?  (I tried making both changes, and any video over 360p still overwhelmed my CPU).

Comment 14

[Dave Yeo]

(In reply to comment #13)
> Dave, looking at your settings for jwasm I see you've set it for a P4.  I don't
> know if it really makes any difference, but isn't that a bit aggressive for our
> userbase?  Perhaps P2 would be more appropriate.

I mostly did that to see if it would help the problems with SSE2, and left it that way due to thinking that the assembler would only assemble the instructions that were presented to it and also it seems I've seen mention of automatic CPU detection. My only assembler experience is with 6502 assembly and little of that.

> 
> Also, I see that there's a setting for the coprocessor which defaults to an
> 8087.  Would changing that to an 80387 help anyone?  (I tried making both
> changes, and any video over 360p still overwhelmed my CPU).

I missed that.
I considered the jwasm as only a temporary solution, I wrestled quite a bit trying to make nasm output 32bit flat OMF object files without success and also played with http://www.agner.org/optimize/#objconv in the hopes of using yasm and converting the ELF to OMF. (emxomfar was never happy with the result) then got distracted trying to figure out the pmmerge crash.

Comment 15

[Dave Yeo]

Attachment: Use nasm as the assembler, aout version

Use nasm -f aout to assemble libvpx. This assembles well with only rodata needing changed to data. As upstream is talking about officially supporting nasm and since yasm is a rewrite of nasm I think this is the way to go unless someone adds aout or/and omf support to yasm. The main syntax difference is that yasm supports gcc style include paths.
I'm not totally happy with using aout but as I have had no success linking with nasm -fobj this will do for now.
I also don't like having the emxomf part in config/rules.mk but could not see an alternative.
Now we need to test the SSE3 parts as my CPU doesn't support it and the multithreading needs to be tested on SMP systems.

Comment 16

[Dave Yeo]

Forgot to add that nasm OS/2 binaries are available at http://nasm.us. I used NASM version 2.08.01-20100422 compiled on Apr 22 2010

Comment 17

[Walter Meinl]

Comment on attachment 452941 [details] [diff] [review]
libvpx - basic OS/2 support - v2

>+++ b/media/libvpx/vpx_config.h
>+#elif defined(__OS2__)
>+/* OS/2 */
>+#include "vpx_config_x86-os2-gcc.h"
>+
> #else
> #error VPX_X86_ASM is defined, but assembly not supported on this platform!
> #endif
>diff --git a/media/libvpx/vpx_config_c.c b/media/libvpx/vpx_config_c.c
>--- a/media/libvpx/vpx_config_c.c
>+++ b/media/libvpx/vpx_config_c.c
>@@ -19,12 +19,12 @@
>-#else
>-#error VPX_X86_ASM is defined, but assembly not supported on this platform!
>-#endif
>+#elif defined(__OS2__)
>+/* OS/2 */
>+#include "vpx_config_x86-os2-gcc.c"

>-
>+#endif
Shouldn't the change in vpx_config_c.c match that in vpx_config.h? An addition of the OS2-lines rather than substituting the error lines?
In addition, I found in vpx_config_x86-os2-gcc.{h,asm} some lines missing as opposed to the other platforms
 CONFIG_PIC equ 0
+CONFIG_BIG_ENDIAN equ 0 
 CONFIG_CODEC_SRCS equ 0
...
+CONFIG_SHARED equ 0  last line

and one different define
CONFIG_INSTALL_SRCS equ 0 instead of
CONFIG_INSTALL_SRCS equ 1 (in the os2 files)

Comment 18

[Walter Meinl]

Comment on attachment 454208 [details] [diff] [review]
Use nasm as the assembler, aout version

diff --git a/media/libvpx/vpx_ports/x86_abi_support.asm b/media/libvpx/vpx_ports/x86_abi_support.asm
--- a/media/libvpx/vpx_ports/x86_abi_support.asm
+++ b/media/libvpx/vpx_ports/x86_abi_support.asm
@@ -226,6 +228,9 @@
 ;
 %ifidn __OUTPUT_FORMAT__,macho64
 %define SECTION_RODATA section .text
+%elifidn __OUTPUT_FORMAT__,obj
+%define SECTION_RODATA section .text
+section .text align=16 use32 flat
 %elifidn __OUTPUT_FORMAT__,macho32
 %macro SECTION_RODATA 0
 section .text
Dave, I looked back at the questions you asked in the newsgroup quite a while ago concerning your problems with NASM and omf file creation. I think I found the right place to add the line with "flat" and so on. I successfully compiled as omf and run a demo webm file on youtube. I changed rodata to .text, but maybe your .data will work as well.

Comment 19

[Dave Yeo]

Good, it looks like the solution was the .text contrary to the documentation and KO Myung-Hun's advice to use SECTION .rodata align=16 class=DATA use32 flat

I just updated the patches re your observations in comment #17 (a stupid mistake on my part and resynching with upstream needing updated) and some refactoring. I'll test with your omf fix and update the patches later.

Comment 20

[Walter Meinl]

(In reply to comment #19)
just another comment to your patch for configure.in
you can add
AS_DASH_C_FLAG='' in your check for NASM right below VPX_ASFLAGS="-f obj"

and I would remove the error if NASM is not found. I think this error is be specific for win, because UNIX allows to build w/o assembler if its not found.

Comment 21

[Rich Walsh]

(In reply to comment #18)

Thanks, Walter.  I was able to build with 'obj' before I saw your comments but the results were really lousy:  everything had a green cast and videos had lots of visual noise.  I think adding 'align=16' made the difference.

One stylistic nit:  it might be more consistent to put the 'section' directive at the end of the file with the other miscellanea, i.e.
 %elifidn __OUTPUT_FORMAT__,elf64
 section .note.GNU-stack noalloc noexec nowrite progbits
 section .text
+%elifidn __OUTPUT_FORMAT__,obj
+section .text align=16 use32 flat
 %endif

Comment 22

[Walter Meinl]

(In reply to comment #21)
> (In reply to comment #18)
> 
> the results were really lousy:  everything had a green cast and videos had lots
> of visual noise.  I think adding 'align=16' made the difference.
Then you were "lucky" having at least an output ;-) When I tried my first builds w/o align=16 with some youtube webm clips the browser immediately crashed.
(In reply to comment #15)
>Now we need to test the SSE3 parts as my CPU doesn't support it and the
>multithreading needs to be tested on SMP systems.
My processor is an Athlon 64 X2, therefore it should be sse3 enabled. Unfortunately, I cannot use SMP mode (though the SMP kernel is installed), because of the AMD chipset on my mainboard.

Comment 23

[Rich Walsh]

I just looked at a map file & realized that the 'section' directive is missing something semi-important. It should be: 
  section .text align=16 use32 flat class=CODE

"class=CODE" will cause the linker to combine the assembler snippets with all of the other code (note that the word 'CODE' must be capitalized).  Without it, they'll be kept separate & be given their own memory segment.  IOW, they would require an additional 64k allocation in the shared memory arena - never a good thing these days.

Comment 24

[Rich Walsh]

(In reply to comment #19)
> Good, it looks like the solution was the .text contrary to the documentation
> and KO Myung-Hun's advice to use SECTION .rodata align=16 class=DATA use32
> flat

Dave, there are 2 somewhat unrelated issues here.  SECTION_RODATA could probably be defined as '.data' with no ill effect.  Declaring it '.text' simply ensures that it really is read-only because it's included with the executable code, and those pages are always r/o.

Walter's 'section .text [...]' directive resolves the linking error you were probably getting.  If NASM has to emit code (or data) and it hasn't been told what segment to put it in, it makes up a name.  All of the .asm files _except_ emms.asm failed to specify a code segment, so they were all assigned to NASM's default name.  However, emms.asm specifies segment .text, so its code went there.

For me, at least, it produced this kind of error:
> error LNK2003: inter-segment self-relative fix-up at 7E0 in segment TEXT32
> target external '_vpx_reset_mmx_state'
Because the calling code was in a different segment than the target, the linker needed enough space to plug in a full address (e.g. 0002:12345678).  However, the assembler assumed all of the code was in the same segment, so it only left room to plug in a relative address (i.e. current address + X bytes).  Adding the 'section .text' directive ensures that everyone's on the same page, so to speak.

Comment 25

[Dave Yeo]

(In reply to comment #24)
> (In reply to comment #19)
> > Good, it looks like the solution was the .text contrary to the documentation
> > and KO Myung-Hun's advice to use SECTION .rodata align=16 class=DATA use32
> > flat
> 
> Dave, there are 2 somewhat unrelated issues here.  SECTION_RODATA could
> probably be defined as '.data' with no ill effect.  Declaring it '.text' simply
> ensures that it really is read-only because it's included with the executable
> code, and those pages are always r/o.

It is good to know that .text is always read-only. IIRC earlier I tried with .data and .rodata and nasm seemed to create another segment which was 16 bit, at least some of the errors were along the lines of you can't mix 16 bit and 32 bit segments. Theoretically .data is better as the linker should automatically do the 16 bit alignment.
Even for the aout I actually meant to use .data and used .text as I was looking at the Mac stuff. 

> 
> Walter's 'section .text [...]' directive resolves the linking error you were
> probably getting.  If NASM has to emit code (or data) and it hasn't been told
> what segment to put it in, it makes up a name.  All of the .asm files _except_
> emms.asm failed to specify a code segment, so they were all assigned to NASM's
> default name.  However, emms.asm specifies segment .text, so its code went
> there.
> 
> For me, at least, it produced this kind of error:
> > error LNK2003: inter-segment self-relative fix-up at 7E0 in segment TEXT32
> > target external '_vpx_reset_mmx_state'
> Because the calling code was in a different segment than the target, the linker
> needed enough space to plug in a full address (e.g. 0002:12345678).  However,
> the assembler assumed all of the code was in the same segment, so it only left
> room to plug in a relative address (i.e. current address + X bytes).  Adding
> the 'section .text' directive ensures that everyone's on the same page, so to
> speak.

OK, that is a good explanation of why I was getting the above error and also that nasm was probably creating another segment with .data.

Comment 26

[Dave Yeo]

Attachment: Basic support to enable libvpx with the dependency on Rich's patch removed

I've made it so this patch should be enough for basic libvpx support minus assembly and multithreading. This patch needs to be applied before the others.

Comment 27

[Dave Yeo]

Attachment: nasm -f obj support

I think I've addressed all the comments with this patch.

Comment 28

[Dave Yeo]

Attachment: Rich's patch, with all the muti-threading stuff from the basic patch.

Just a minor rearranging so that the compile will work without this patch applied.

Comment 29

[Rich Walsh]

(In reply to comment #28)
> Created an attachment (id=454329) [details]
> Rich's patch, with all the muti-threading stuff from the basic patch.
> 
> Just a minor rearranging so that the compile will work without this patch
> applied.

I don't understand why you're making what seems to me to be an artificial distinction.  Are you saying that multithreading shouldn't be standard?  If not, then why?  The only reason I can see for not making this all one patch is to separate Mozilla implementation details (i.e. configure.in) from changes to libvpx itself.

Comment 30

[Dave Yeo]

(In reply to comment #29)
> (In reply to comment #28)
> > Created an attachment (id=454329) [details] [details]
> > Rich's patch, with all the muti-threading stuff from the basic patch.
> > 
> > Just a minor rearranging so that the compile will work without this patch
> > applied.
> 
> I don't understand why you're making what seems to me to be an artificial
> distinction.  Are you saying that multithreading shouldn't be standard?  If
> not, then why?  The only reason I can see for not making this all one patch is
> to separate Mozilla implementation details (i.e. configure.in) from changes to
> libvpx itself.

Yes, I was thinking about keeping the various implementation details separate.
Also it is generally easier to break the patches and submit one at a time upstream (with a lot of projects). Some projects are happy to try to support everything and others consider OS/2 not worth supporting (x264 refused any OS/2 patches). The patch submission process is also quite different from anything I have experience with, see http://www.webmproject.org/code/contribute/submitting-patches/

Comment 31

[Rich Walsh]

I'm attaching updates to the multithread and basic support patches.

The change to the multithread patch was required by a recent version upgrade in libvpx.  The change to the basic support patch is more stylistic than substantive.

The previous version prevented the inclusion of semaphore.h like this:
  +#if CONFIG_MUTITHREAD
   #include <semaphore.h>
  +#endif

Since this #include was never going to be valid on OS/2, regardless of whether multithreading was enabled, it seemed inappropriate.  Instead, I removed it and replaced it with this placeholder:
  +#else
  +#ifdef __OS2__
  +/* this is a no-op until OS/2 threading support is added */
   #else
   [...]
   #include <semaphore.h>
   [...]

In the multithread patch, the comment is replaced by the required #includes and #defines.

FWIW... To ensure I had put my #ifdefs and #endifs in the right places in threading.h, I simulated each of the other build environments and got the errors I would expect.

Comment 32

[Rich Walsh]

Attachment: libvpx basic support - v4

Comment 33

[Rich Walsh]

Attachment: libvpx multithread support - v3

Comment 34

[Rich Walsh]

Attachment: libvpx multithread support - v3a

bitrot update

Comment 35

[Rich Walsh]

Attachment: libvpx assembler support - v3

accommodates changes in configure.in since previous patch and ensures the linker puts assembled code in the same segment as c/c++ code

Comment 36

[Chris Peterson [:cpeterson]]

WONTFIX because OS/2 is no longer a supported platform.