liboggplay cpuid checks broken for Windows AMD64

RESOLVED FIXED

Status

()

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

People

(Reporter: dvander, Unassigned)

Tracking

unspecified
x86_64
Windows NT
Points:
---

Firefox Tracking Flags

(status1.9.2 beta1-fixed, status1.9.1 .4-fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
Created attachment 401604 [details] [diff] [review]
proposed fix

Microsoft's x64 compiler does not support inline assembly, we must use the __cpuid intrinsic instead. Additionally x64 CPUs will always have CPUID, so there's no need to perform a runtime check.
Attachment #401604 - Flags: review?(chris.double)
(Reporter)

Updated

8 years ago
Hardware: x86 → x86_64
cpu.c was cribbed from libtheora, so upstream might want these fixes too.  We're we're not hitting this case at build time in libtheora because we build without USE_ASM of x86-64.

Comment 2

8 years ago
The patch needs to be updated to include a .patch file that can be used to re-apply it when we update to new versions of libtheora, a line added to update.sh to apply the patch and a description added to README_MOZILLA referring to this bug.

See how other patches have been done in the various /media subdirectories for examples.

Updated

8 years ago
Attachment #401604 - Flags: review?(chris.double) → review+

Comment 3

8 years ago
Comment on attachment 401604 [details] [diff] [review]
proposed fix

r+ with changes mentioned in comment 2
This bug is already merged on upstream.

http://git.xiph.org/?p=liboggplay.git;a=commitdiff;h=b908ed69267c001310cc234628fb4c7dc41ced76
Was this fixed by bug 512328? I updated liboggplay yesterday.
Depends on: 512328
(In reply to comment #5)
> Was this fixed by bug 512328? I updated liboggplay yesterday.

Yes.  About cpu.c issue, this is fixed.

But I found other problem. (I found simple typo bug of x64 issue in yuv2rgb_x86_vs.h).  About it, I will file a bug to xiph.org.
Fixed on trunk and 1.9.2 by bug 512328 checkin.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
status1.9.2: --- → beta1-fixed
Resolution: --- → FIXED
Fixed on trunk and 1.9.1 by bug 512328 checkin.
status1.9.1: --- → .4-fixed
You need to log in before you can comment on or make changes to this bug.