Closed Bug 1037125 Opened 6 years ago Closed 6 years ago

Basic automated tests for GMP plugins

Categories

(Core :: WebRTC: Audio/Video, defect)

x86_64
Linux
defect
Not set
Points:
8

Tracking

()

RESOLVED FIXED
mozilla33
Iteration:
33.3

People

(Reporter: benjamin, Assigned: jesup)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

We need an in-tree GMP plugin for testing purposes, and we need to exercize it from automated tests at least enough to know that the GMP code works, loads plugins, and doesn't leak/assert. I think we're going to try and model this after the plugin tests.

ekr promised me some sample code for a GMP. Once that's available, please reassign the bug to me and I'll hook it up into mochitests.
bsmedberg,

Here is a WIP fake plugin. It pretends to be an H.264 codec, but just compresses
each frame down to a quasi-average of all the pixels in the frame. You should
be able to use it in place of the official plugin.

Notes:
1. As I said, this is a WIP. Cleanup needed.
2. You may want to move the directory it is in. I wanted it in media/webrtc, but
ran into problems with the gyp/mozbuild interaction
3. You need to put the .info and .dylib files somewhere, as usual.
Assignee: ekr → benjamin
Hi Benjamin, wanted to confirm if you are adding this to Iteration 33.3?
Flags: needinfo?(benjamin)
Flags: firefox-backlog+
QA Whiteboard: [qa-]
Flags: needinfo?(benjamin)
Depends on: 1040345
Attached patch use-testpluginSplinter Review
gps, review mainly for the build bits of getting the GMP plugin into the chrome-mochitest directory. I couldn't think of a better way.
Attachment #8458273 - Flags: review?(gps)
Comment on attachment 8455061 [details] [diff] [review]
Fake GMP plugin

Jesup could you review this? In particular, I want to make sure that this is code that we can legally distribute and it makes sense to do it like this.
Attachment #8455061 - Flags: review?(rjesup)
Blocks: 1040346
Comment on attachment 8455061 [details] [diff] [review]
Fake GMP plugin

Review of attachment 8455061 [details] [diff] [review]:
-----------------------------------------------------------------

r+.  We won't be shipping this, but i'd run it by Gerv on general principle, but there's nothing wrong there.

::: dom/media/gmp-plugin/fake.info
@@ +1,4 @@
> +Name: fake
> +Description: Fake GMP Plugin
> +Version: 1.0
> +APIs: encode-video[h264:vp8], decode-video[h264:vp8]

h264 instead of h264:vp8
Attachment #8455061 - Flags: review?(rjesup) → review+
Comment on attachment 8458273 [details] [diff] [review]
use-testplugin

Review of attachment 8458273 [details] [diff] [review]:
-----------------------------------------------------------------

The Makefile.in is sane. But you should define this autogenerated file in chrome.ini under support-files and generated-files. This will prevent the file from being deleted at the top of the build.

https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/buildsystem/test_manifests.html#recognized-metadata
Attachment #8458273 - Flags: review?(gps) → feedback+
FYI, we're writing gtests over in bug 1039886.
gps or other build-people, can generated-files contain gmp-fake/* or can I use variables like LIB_PREFIX/LIB_SUFFIX in chrome.ini to get the library name?
Flags: needinfo?(gps)
Ted -- GPS is on PTO, and I'm told you're the best person to ask re:Comment 9 (above) from bsmedberg.
Flags: needinfo?(ted)
The logic that handles installing support-files/generated-files from moz.build land is here:
http://hg.mozilla.org/mozilla-central/annotate/e6e8c93a1b6e/python/mozbuild/mozbuild/frontend/emitter.py#l542

and the bit that actually produces InstallManifests from that is here:
http://hg.mozilla.org/mozilla-central/annotate/330ba968ed61/python/mozbuild/mozbuild/backend/recursivemake.py#l1044

It looks like while we support wildcards in support-files, we won't handle them properly in generated-files. We also don't currently have any way to preprocess manifests or use variables in filenames.

However, you can still just copy files the old-fashioned way from a Makefile into a test directory and have it work anyway, since we don't have 100% conversion of files into manifests anyway, like so:
http://hg.mozilla.org/mozilla-central/annotate/330ba968ed61/toolkit/components/ctypes/tests/Makefile.in#l12
Flags: needinfo?(ted)
Flags: needinfo?(gps)
Handing this off to Jesup. After discussion, the second patch here is inefficient but should be correct.
Assignee: benjamin → rjesup
Iteration: 33.3 → ---
Attachment #8459195 - Flags: review?(mh+mozilla)
Comment on attachment 8458273 [details] [diff] [review]
use-testplugin

Review of attachment 8458273 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/test/test_fake_h264.html
@@ +52,5 @@
> +
> +    // This sucks. We don't know when the registration will finish on the GMP
> +    // thread, but we need to wait for it. Do something better... have
> +    // addPluginDirectory return a promise, maybe?
> +    setTimeout(start, 500);

Mid-/Long-term it might be useful to have some kind of "gmp-plugin-added" notifications for consumers like this or addons/frontend-features/...
Blocks: 1041347
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a186f354cd6
https://hg.mozilla.org/integration/mozilla-inbound/rev/662c3668db6e

Note: the fake plugin gets installed to dist/bin when building, but isn't packaged yet and thus isn't available in automated tests (manual tests work).  See bug 1041347
Target Milestone: --- → mozilla33
https://hg.mozilla.org/mozilla-central/rev/8a186f354cd6
https://hg.mozilla.org/mozilla-central/rev/662c3668db6e
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Iteration: --- → 33.3
You need to log in before you can comment on or make changes to this bug.