+++ This bug was initially created as a clone of Bug #1223692 +++ https://groups.google.com/a/webmproject.org/d/msg/codec-devel/Vt6c2Z96cWI/aTYeP5y6BwAJ https://chromium.googlesource.com/webm/libvpx/+/v1.6.1 2017-01-09 v1.6.1 "Long Tailed Duck" This release improves upon the VP9 encoder and speeds up the encoding and decoding processes. - Upgrading: This release is ABI compatible with 1.6.0. - Enhancements: Faster VP9 encoding and decoding. High bit depth builds now provide similar speed for 8 bit encode and decode for x86 targets. Other platforms and higher bit depth improvements are in progress. - Bug Fixes: A variety of fuzzing issues.

I've run into a bit of an issue. Upstream created a "header" file for the neon assembly for some common macros. Unfortunately it still needs to be run through ads2gas.pl I see two ways to fix this: 1) pre-run it through ads2gas.pl because FF only supports *nix builds so I don't need to care about whether to use ads2gas.pl or ads2gas_apple.pl 2) Add the generated assembly directory to ASFLAGS with -I but I don't know what variable to use for that directory. The file in question is obj-arm-linux-androideabi/media/libvpx/libvpx/vpx_dsp/arm/idct_neon.asm.S and is include with the full path, so I need something like ASFLAGS += -Iobj-arm-linux-androideabi/media/libvpx/libvpx/ Preference for either approach? Suggested moz.build variable for the include path?

I think running the generator sounds safer. You can add paths to the build with the `LOCAL_INCLUDES` variable (a list) in moz.build. You can use `OBJDIR` to reference the target directory for generated files. https://gecko.readthedocs.io/en/latest/build/buildsystem/mozbuild-symbols.html#objdir

Apologies for the bz2. The patch went over the 10mb limit. (In reply to Ralph Giles (:rillian) | needinfo me from comment #2) > You can add paths to the build with the `LOCAL_INCLUDES` variable (a list) Does that include ASFLAGS? It didn't appear to from the documentation. If it does (or even just for CFLAGS) there are a number of places in media/libvpx/moz.build that should probably use it. Unfortunately I don't really have a good setup for experimenting and testing different build rules. > in moz.build. You can use `OBJDIR` to reference the target directory for Thanks. I was able to use OBJDIR

(In reply to johannkoenig from comment #3) > > You can add paths to the build with the `LOCAL_INCLUDES` variable (a list) > > Does that include ASFLAGS? Looks like it gets appended after ASFLAGS. Of course you can use ASFLAGS if you you want the include to be specific to the assembler. https://dxr.mozilla.org/mozilla-central/source/config/rules.mk#983 > Thanks. I was able to use OBJDIR Great.

Comment on attachment 8827535 [details] 0001-Bug-1331498-Update-libvpx-to-v1.6.1.patch.bz2 Moving to reviewboard.

Comment on attachment 8827599 [details] Bug 1331498: Update libvpx to v1.6.1. https://reviewboard.mozilla.org/r/105228/#review106044 ::: media/libvpx/rename_duplicate_files.patch:4 (Diff revision 1) > diff --git a/libvpx/vpx_dsp/x86/loopfilter_sse2.c b/libvpx/vpx_dsp/x86/loopfilter_intrin_sse2.c > similarity index 100% > rename from libvpx/vpx_dsp/x86/loopfilter_sse2.c > rename to libvpx/vpx_dsp/x86/loopfilter_intrin_sse2.c When I applied your 1.6.0 patch this worked, but today when I try running update.py on my mac with this patch, this rename doesn't happen, and this part of the diff ends up reverted. I guess Apple's `patch` doesn't implement the rename syntax. Not sure how best to address this: vcs moves are good, but it's bad that the script doesn't work on Mac. Maybe the best thing is to replace this section of the patch with an explicit `os.system('mv ..')` in update.py? ::: media/libvpx/update.py:49 (Diff revision 1) > def update_readme(commit): > with open('README_MOZILLA') as f: > readme = f.read() > > if 'The git commit ID used was' in readme: > new_readme = re.sub('The git commit ID used was [a-f0-9]+', Sorry, I didn't catch this in the previous commit, but the regex no longer works now that we have release tags we can update against. (This is a good thing!) Can you change this and re-run the update so README_MOZILLA gets updated properly? Just `[v\.a-f0-9]+` would be sufficient for the release tags along with commit hashes. Or a general 'any string without whitespace' is probably fine too.

Comment on attachment 8827599 [details] Bug 1331498: Update libvpx to v1.6.1. https://reviewboard.mozilla.org/r/105228/#review106052 Looks good in general but needs a few fixes. Please address issues and update the patch. ::: media/libvpx/moz.build:50 (Diff revision 1) > arm_asm_files = files['ARM_SOURCES'] > > if CONFIG['VPX_AS_CONVERSION']: > SOURCES += sorted([ > - "!%s.s" % f if f.endswith('.asm') else f for f in arm_asm_files > + "!%s.S" % f if f.endswith('.asm') else f for f in arm_asm_files > ]) This seems to be failing in the arm builds. Maybe looking in the source instead of the build directory for the translated file? ``` /usr/bin/perl /builds/slave/try-and-api-15-000000000000000/build/src/media/libvpx/libvpx/build/make/ads2gas.pl < /builds/slave/try-and-api-15-000000000000000/build/src/media/libvpx/libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm > libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm.S /bin/sh: libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm.S: No such file or directory gmake[5]: *** [libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm.S] Error 1 ```

(In reply to Ralph Giles (:rillian) | needinfo me from comment #7) > > rename from libvpx/vpx_dsp/x86/loopfilter_sse2.c > > rename to libvpx/vpx_dsp/x86/loopfilter_intrin_sse2.c > > I guess Apple's `patch` doesn't implement the rename syntax. Switched to os.system("mv ...") > Sorry, I didn't catch this in the previous commit, but the regex no longer > works now that we have release tags we can update against. (This is a good > thing!) Huh, I guess I updated it manually. Fixed. (In reply to Ralph Giles (:rillian) | needinfo me from comment #8) > This seems to be failing in the arm builds. Maybe looking in the source in media/libvpx/Makefile.in: GENERATED_DIRS += $(dir $(ASFILES)) .S files are not added to ASFILES. This doesn't appear to affect the build in any other way. Forced the directory creation with: GENERATED_DIRS += libvpx/vpx_dsp/arm

Comment on attachment 8827616 [details] 0001-Bug-1331498-Update-libvpx-to-v1.6.1.patch.bz2 Pushed to reviewboard.

Comment on attachment 8827599 [details] Bug 1331498: Update libvpx to v1.6.1. https://reviewboard.mozilla.org/r/105228/#review106082 lgtm. r+ assuming the android try push is green.

Comment on attachment 8827599 [details] Bug 1331498: Update libvpx to v1.6.1. https://reviewboard.mozilla.org/r/105228/#review106052 > This seems to be failing in the arm builds. Maybe looking in the source instead of the build directory for the translated file? > ``` > /usr/bin/perl /builds/slave/try-and-api-15-000000000000000/build/src/media/libvpx/libvpx/build/make/ads2gas.pl < /builds/slave/try-and-api-15-000000000000000/build/src/media/libvpx/libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm > libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm.S > /bin/sh: libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm.S: No such file or directory > gmake[5]: *** [libvpx/vpx_dsp/arm/idct16x16_1_add_neon.asm.S] Error 1 > ``` This is fixed with the `GENERATED_DIRS` change.

Pushed by rgiles@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/58764883978d Update libvpx to v1.6.1. r=rillian