Status

()

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

People

(Reporter: derf, Assigned: derf)

Tracking

Trunk
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec2.0b3+)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 486704 [details] [diff] [review]
Update libvpx to v0.9.5

This update includes additional 20-40% speed improvements, as well as runtime CPU detection for ARM, and some additional bug fixes.

The primary purpose of taking this update is to add support for the optimized ARM asm, which requires the runtime CPU detection. Because there have been significant changes to the ARM asm since the last release (and adding runtime CPU detection required significant additional changes), the best approach is to update the whole library.

This update also allows us to drop all of our custom patches except the Solaris Sun Studio one.
Attachment #486704 - Flags: review?(chris)
(Assignee)

Comment 1

8 years ago
Created attachment 486798 [details] [diff] [review]
Update libvpx to v0.9.5 (take 2)

Original patch was missing two files.
Assignee: nobody → tterribe
Attachment #486704 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #486798 - Flags: review?(chris)
Attachment #486704 - Flags: review?(chris)
(Assignee)

Comment 2

8 years ago
Created attachment 486836 [details] [diff] [review]
Update libvpx to v0.9.5 (take 3)

Apparently Android doesn't #define __linux__ (or anything else to indicate the OS, at all), so give it a little help in the Makefile.
Attachment #486798 - Attachment is obsolete: true
Attachment #486836 - Flags: review?(chris)
Attachment #486798 - Flags: review?(chris)
Comment on attachment 486836 [details] [diff] [review]
Update libvpx to v0.9.5 (take 3)

Looks good. Builds and tests pass on all platforms on TryServer.

We should get one of the build guys (such as Ted Mielczarek or khuey) to do an additional review of the build/configure stuff as well.
Attachment #486836 - Flags: review?(chris) → review+
Comment on attachment 486836 [details] [diff] [review]
Update libvpx to v0.9.5 (take 3)

r+, just a few nits.

>@@ -6052,24 +6058,37 @@ if test -n "$MOZ_WEBM"; then
>         elif test -n "$COMPILE_ENVIRONMENT" -a "$_YASM_MAJOR_VERSION" -lt "1" -o \( "$_YASM_MAJOR_VERSION" -eq "1" -a "$_YASM_MINOR_VERSION" -lt "1" \) ; then
>           AC_MSG_ERROR([yasm 1.1 or greater is required to build libvpx on Win32, but you appear to have version $_YASM_MAJOR_VERSION.$_YASM_MINOR_VERSION.  Upgrade to the newest version (included in MozillaBuild 1.5.1 and newer) or configure with --disable-webm (which disables the WebM video format). See https://developer.mozilla.org/en/YASM for more details.])
>         else
>           VPX_ASFLAGS="-f win32 -rnasm -pnasm -DPIC"
>           VPX_X86_ASM=1
>         fi
>       fi
>     ;;
>+    *:arm*)
>+      if test -n "$GNU_AS" ; then
>+        VPX_AS=$AS
>+        dnl These flags are a lie; they're just used to enable the requisite
>+        dnl  opcodes; actual arch detection is done at runtime.
Inconsistent spacing

>@@ -191,17 +205,127 @@ ASFILES += \
>   subpixel_mmx.asm \
>   subpixel_sse2.asm \
>   subpixel_ssse3.asm \
>   dequantize_mmx.asm \
>   emms.asm \
>   $(NULL)
> 
> endif
>- 
>+
>+ifdef VPX_ARM_ASM
>+# Building on an ARM platform with a supported assembler, include
>+# the optimized assembly in the build.
>+
>+# The Android NDK doesn't pre-define anything to indicate the OS it's on, so
>+#  do it for them.
Inconsistent spacing
>+ifeq ($(OS_TARGET),Android)
>+DEFINES += -D__linux__
>+endif
>+
>+CSRCS += \
>+  arm_cpudetect.c \
>+  arm_systemdependent.c \
>+  bilinearfilter_arm.c \
>+  filter_arm.c \
>+  loopfilter_arm.c \
>+  reconintra_arm.c \
>+  arm_dsystemdependent.c \
>+  dequantize_arm.c \
>+  idct_blk_v6.c \
>+  idct_blk_neon.c \
>+  recon_neon.c \
>+  $(NULL)
>+
>+VPX_ASFILES = \
>+  detokenize.asm \
>+  bilinearfilter_v6.asm \
>+  copymem8x4_v6.asm \
>+  copymem8x8_v6.asm \
>+  copymem16x16_v6.asm \
>+  dc_only_idct_add_v6.asm \
>+  iwalsh_v6.asm \
>+  filter_v6.asm \
>+  idct_v6.asm \
>+  loopfilter_v6.asm \
>+  recon_v6.asm \
>+  simpleloopfilter_v6.asm \
>+  sixtappredict8x4_v6.asm \
>+  bilinearpredict4x4_neon.asm \
>+  bilinearpredict8x4_neon.asm \
>+  bilinearpredict8x8_neon.asm \
>+  bilinearpredict16x16_neon.asm \
>+  copymem8x4_neon.asm \
>+  copymem8x8_neon.asm \
>+  copymem16x16_neon.asm \
>+  dc_only_idct_add_neon.asm \
>+  iwalsh_neon.asm \
>+  loopfilter_neon.asm \
>+  loopfiltersimplehorizontaledge_neon.asm \
>+  loopfiltersimpleverticaledge_neon.asm \
>+  mbloopfilter_neon.asm \
>+  recon2b_neon.asm \
>+  recon4b_neon.asm \
>+  reconb_neon.asm \
>+  shortidct4x4llm_1_neon.asm \
>+  shortidct4x4llm_neon.asm \
>+  sixtappredict4x4_neon.asm \
>+  sixtappredict8x4_neon.asm \
>+  sixtappredict8x8_neon.asm \
>+  sixtappredict16x16_neon.asm \
>+  recon16x16mb_neon.asm \
>+  buildintrapredictorsmby_neon.asm \
>+  save_neon_reg.asm \
>+  dequant_dc_idct_v6.asm \
>+  dequant_idct_v6.asm \
>+  dequantize_v6.asm \
>+  idct_dequant_dc_full_2x_neon.asm \
>+  idct_dequant_dc_0_2x_neon.asm \
>+  dequant_idct_neon.asm \
>+  idct_dequant_full_2x_neon.asm \
>+  idct_dequant_0_2x_neon.asm \
>+  dequantizeb_neon.asm \
>+  $(NULL)
>+
>+#The ARM asm needs to extract the offsets of various C struct members.
>+#We need a program that runs on the host to pull them out of a .o file.
More inconsistent spacing
>+HOST_CSRCS = obj_int_extract.c
>+HOST_PROGRAM = obj_int_extract$(HOST_BIN_SUFFIX)

Typically we like to make host programs take the form "host_foo$(HOST_BIN_SUFFIX)".  I thought the build system enforced that, but I can't find that code anywhere.

>+
>+ifdef VPX_AS_CONVERSION
>+# The ARM asm is written in ARM RVCT syntax, but we actually build it with
>+#  gas using GNU syntax.
>+# Add some rules to perform the conversion.
And some more
>+VPX_CONVERTED_ASFILES = $(addsuffix .$(ASM_SUFFIX), $(VPX_ASFILES))
>+
>+ASFILES += $(VPX_CONVERTED_ASFILES)
>+GARBAGE += $(VPX_CONVERTED_ASFILES)
>+
>+%.asm.$(ASM_SUFFIX): %.asm
>+	$(VPX_AS_CONVERSION) < $< > $@
>+
>+vpx_asm_offsets.asm: vpx_asm_offsets.$(OBJ_SUFFIX) $(HOST_PROGRAM)
>+	./$(HOST_PROGRAM) rvds $< | $(VPX_AS_CONVERSION) > $@
>+
>+detokenize.asm.$(OBJ_SUFFIX): vpx_asm_offsets.asm
>+
>+else
>+ASFILES += $(VPX_ASFILES)
>+
>+vpx_asm_offsets.asm: vpx_asm_offsets.$(OBJ_SUFFIX) $(HOST_PROGRAM)
>+	./$(HOST_PROGRAM) rvds $< > $@
>+
>+detokenize.$(OBJ_SUFFIX): vpx_asm_offsets.asm
>+
>+endif
>+
>+GARBAGE += vpx_asm_offsets.$(OBJ_SUFFIX) vpx_asm_offsets.asm
>+
>+endif
>+
> include $(topsrcdir)/config/rules.mk
> 
> # Workaround a bug of Sun Studio (CR 6963410)
> ifdef SOLARIS_SUNPRO_CC
> ifeq (86,$(findstring 86,$(OS_TEST)))
> filter_c.o: filter_c.c Makefile.in
> 	$(REPORT_BUILD)
> 	@$(MAKE_DEPS_AUTO_CC)
Attachment #486836 - Flags: review+
(Assignee)

Comment 5

8 years ago
Created attachment 487046 [details] [diff] [review]
Update libvpx to v0.9.5 (take 4)

Addressed review comments. Carrying forward r=cpearce,khuey

This builds and runs for me on x86-64 Linux, Maemo 5, and Android 2.2.
Attachment #486836 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
tracking-fennec: --- → ?

Updated

8 years ago
tracking-fennec: ? → 2.0b3+
Whiteboard: [has-patch]

Updated

8 years ago
Duplicate of this bug: 577204
http://hg.mozilla.org/mozilla-central/rev/2ef1a570e14e
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [has-patch]
This breaks builds from relative srcdirs, e.g. '../mozilla-central/configure' due to VPX_AS_CONVERSION being defined here using ${srcdir}:

  http://mxr.mozilla.org/mozilla-central/source/configure.in#6108

${srcdir} is the dir during configure execution, so it would be "../mozilla-central" which isn't valid when this is executed from "media/libvpx".
Comment on attachment 489957 [details] [diff] [review]
srcdir location fix

nvm, mwu already caught and fixed this
Attachment #489957 - Attachment is obsolete: true
Attachment #489957 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

8 years ago
Blocks: 604992
You need to log in before you can comment on or make changes to this bug.