The default bug view has changed. See this FAQ.

Enable VP8 asm on x86-64 Linux

RESOLVED FIXED in mozilla2.0b1

Status

()

Core
Audio/Video
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: derf, Assigned: derf)

Tracking

unspecified
mozilla2.0b1
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.23) Gecko/20100611 SeaMonkey/1.1.18
Build Identifier: 

Asm for VP8 was previously disabled on x86-64 Linux due to linking problems. These problems should be resolved and asm enabled on this platform.

Reproducible: Always
(Assignee)

Comment 1

7 years ago
Created attachment 452084 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.
Attachment #452084 - Flags: review?(chris)
Attachment #452084 - Attachment is patch: true
Attachment #452084 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 452084 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

The ":data hidden" directive breaks the yasm2masm.py translator/ml.exe wrapper:

YASM-to-MASM translate-compile: /d/work/src/purple/media/libvpx/vp8/common/x86/subpixel_mmx.asm
ml -c -Fo subpixel_mmx.obj  -I/d/work/src/purple/media/libvpx/ -I/d/work/src/purple/media/libvpx/vpx_ports/ -D __OUTPUT_FORMAT__=win32 -safeseh subpixel_mmx.obj
.p
Microsoft (R) Macro Assembler Version 9.00.30729.01
Copyright (C) Microsoft Corporation.  All rights reserved.

 Assembling: subpixel_mmx.obj.p
subpixel_mmx.obj.p(735) : error A2008:syntax error : :
subpixel_mmx.obj.p(795) : error A2008:syntax error : :
make[1]: *** [subpixel_mmx.obj] Error 1

Can you make the ":data hidden" into a preprocessor macro which only has a body on x86_64 Linux? Or find some other way to only have it present on x86_64 Linux? Thanks!
Attachment #452084 - Flags: review?(chris) → review-
(Assignee)

Comment 3

7 years ago
Created attachment 452099 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

This version uses a macro to only apply the :data hidden directive for ELF64 targets.
Attachment #452084 - Attachment is obsolete: true
Attachment #452099 - Flags: review?(chris)
Comment on attachment 452099 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

I've pushed to tryserver, and have also started x86 linux, mac, and win32 builds locally with this patch applied. You need to add the following to your patch, to compile on Win32. If you add that, and it builds on the platforms I'm testing, I'm happy to r+ it.

diff --git a/media/libvpx/vpx_ports/x86_abi_support_win32.asm b/media/libvpx/vpx_ports/x86_abi_support_win32.asm
--- a/media/libvpx/vpx_ports/x86_abi_support_win32.asm
+++ b/media/libvpx/vpx_ports/x86_abi_support_win32.asm
@@ -61,13 +61,15 @@ SHADOW_ARGS_TO_STACK macro v
                      endm
 UNSHADOW_ARGS macro v
               endm

 SECTION_RODATA macro
                .const
                endm

+HIDDEN_DATA textequ <>
+
 .686p
 .XMM
 .model flat, C
 option casemap :none    ; be case-insensitive
 .code
Attachment #452099 - Flags: review?(chris) → review-
(In reply to comment #4)
> (From update of attachment 452099 [details] [diff] [review])
> I've pushed to tryserver, and have also started x86 linux, mac, and win32
> builds locally with this patch applied. 

Local x86 Linux and Win32 builds succeeded (with the addition from comment 4). Mac and TryServer will take longer, to complete.
(Assignee)

Comment 6

7 years ago
Created attachment 452117 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

Add HIDDEN_DATA macro for masm win32 build.
Attachment #452099 - Attachment is obsolete: true
Attachment #452117 - Flags: review?(chris)
Assignee: nobody → tterribe
Status: UNCONFIRMED → NEW
Ever confirmed: true
Add bug URL on Issue tracker of libvpx
Attachment #452117 - Flags: review?(chris) → review+
(Assignee)

Comment 8

7 years ago
This has been submitted upstream as
http://review.webmproject.org/187
Keywords: checkin-needed
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/2f0443835b3d
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9.3a6
You need to log in before you can comment on or make changes to this bug.