+++ This bug was initially created as a clone of Bug #677653 +++ See bug 677653 comment 6 for a lead on how do this for libvpx.

This doesn't really depend on any of the vorbis stuff in bug 677653 landing. But it does depend on figuring out how to intelligently export the libvpx header files. Trying |#include "vpx/vpx_mem.h"| from anywhere not in media/libvpx/ gets you: In file included from /home/froydnj/src/gecko-dev.git/xpcom/build/nsXPComInit.cpp:133:0, from /opt/build/froydnj/build-mc/xpcom/build/Unified_cpp_xpcom_build2.cpp:67: ../../dist/include/vpx/vpx_mem.h:15:24: fatal error: vpx_config.h: No such file or directory compilation terminated. I guess we also want to export vpx_config.h and all its subheaders? Though it looks like vpx_config.h isn't really set up to provide configuration in a platform-generic manner, e.g. you'll only get a correct configuration for Windows if VPX_X86_ASM is defined, which seems unusual.

I guess we need to export vpx_config* if we want the other headers to be useful. Alternatively, you could LOCAL_INCUDES += ['/media/libvpx'] and #include <vpx_mem/vpx_mem.h> instead. Or put the memory reporter in the libvpx tree. > Though it looks like vpx_config.h isn't really set up to provide configuration in a platform-generic manner This is all a hack because mach doesn't support calling external build systems. Rather than maintain a completely parallel build description, our import script runs the upstream configure for each target, copies that into our tree as a platform-specific header, and then includes the appropriate one from a vpx_config.h wrapper. IIRC the asm-platform inversion is because we don't have a platform CPP define for android, and the platform-specific headers assume asm is available. Not saying it can't be cleaned up, of course.

(In reply to Ralph Giles (:rillian) from comment #2) > Alternatively, you could LOCAL_INCUDES += ['/media/libvpx'] and #include > <vpx_mem/vpx_mem.h> instead. Or put the memory reporter in the libvpx tree. Putting the memory reporter in the libvpx tree defeats the purpose of having vpx_mem_set_functions. :) The LOCAL_INCLUDES thing seems plausible, I guess; xpcom/build already pulls in a bunch of different directories, what's one more? > > Though it looks like vpx_config.h isn't really set up to provide configuration in a platform-generic manner > > IIRC the asm-platform inversion is because we don't have a platform CPP > define for android, and the platform-specific headers assume asm is > available. Not saying it can't be cleaned up, of course. __ANDROID__ doesn't work?

This patch is based on top of the patches in bug 677653 for convenience. Nick for the memory reporting bits, Ted for the build bits, and Benjamin to confirm that we're OK calling out to libogg and libvpx from xpcom/build/ (which I guess should really cover the patches in bug 677653 too..).

Comment on attachment 8382244 [details] [diff] [review] add a memory reporter for memory from libvpx What library is vpx_mem_set_functions in? If gkmedias.dll will it hurt startup speed to load that here instead of later when we actually need it? Otherwise this seems fine. Also does this need a feature ifdef? Are there configurations that don't have vpx?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #5) > Comment on attachment 8382244 [details] [diff] [review] > add a memory reporter for memory from libvpx > > What library is vpx_mem_set_functions in? If gkmedias.dll will it hurt > startup speed to load that here instead of later when we actually need it? > Otherwise this seems fine. Yes, it's in gkmedias. I haven't tested to see whether it will affect startup speed. I don't know whether there's a good place for the vpx_mem_set_functions call to ensure that it happens prior to any libvpx memory allocation. Perhaps someplace in content/media/? ni?'ing :cpearce for that. > Also does this need a feature ifdef? Are there configurations that don't > have vpx? Doh, yes. I did the MOZ_VPX dance for the #include, but failed to do it for everything else. =/

Comment on attachment 8382244 [details] [diff] [review] add a memory reporter for memory from libvpx Experimentation with the ogg patch suggests that even asking XPCOM to access gkmedias this early is causing problems. Canceling reviews for now until things get sorted.

I'm no expert on libvpx, but I'd guess we'd need to set memory reporters before all calls to vpx_codec_dec_init or vpx_codec_vp8_dx() or both... Note we use libvpx inside the WebRTC code too.

OK, the gkmedias bits have been sorted out along with the MOZ_VPX bits. This patch is ready for review. It applies on top of the patches in bug 677653. Nick for the memory reporting bits, Ted for the moz.build bits.

Landed with a simple change to symbols.def.in so vpx_mem_set_functions is visible outside gkmedias.dll: https://hg.mozilla.org/integration/mozilla-inbound/rev/3719528f8252