Closed Bug 608066 Opened 9 years ago Closed 9 years ago

Update libvpx to v0.9.5

Categories

(Core :: Audio/Video, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0b3+ ---

People

(Reporter: derf, Assigned: derf)

References

Details

Attachments

(1 file, 4 obsolete files)

Attached patch Update libvpx to v0.9.5 (obsolete) — Splinter Review
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)
Attached patch Update libvpx to v0.9.5 (take 2) (obsolete) — Splinter Review
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)
Attached patch Update libvpx to v0.9.5 (take 3) (obsolete) — Splinter Review
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+
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
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0b3+
Whiteboard: [has-patch]
Duplicate of this bug: 577204
http://hg.mozilla.org/mozilla-central/rev/2ef1a570e14e
Status: ASSIGNED → RESOLVED
Closed: 9 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)
You need to log in before you can comment on or make changes to this bug.