Closed Bug 1159300 Opened 9 years ago Closed 9 years ago

GMP OpenH264 fails to decode on reload on Windows

Categories

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

38 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- unaffected
firefox38 + verified
firefox38.0.5 + fixed
firefox39 + fixed
firefox40 + fixed
firefox-esr38 38+ verified

People

(Reporter: ehugg, Assigned: jesup)

Details

(Keywords: regression)

Attachments

(6 files, 5 obsolete files)

3.53 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
3.53 KB, patch
eflores
: review+
Details | Diff | Splinter Review
10.77 KB, patch
glandium
: review+
jesup
: review+
Details | Diff | Splinter Review
10.90 KB, patch
Details | Diff | Splinter Review
3.69 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
1.50 KB, patch
eflores
: review+
Details | Diff | Splinter Review
Running this test on Windows with FF38-40:
https://mozilla.github.io/webrtc-landing/pc_test.html
with "Require H.264 video" selected, hit start and it works, but if you hit stop and then start again it does not.  On the second time GMP logs that OpenH264 is encoding but not decoding so the video screen remain blank.

This does not happen with FF38-40 on OSX, only on Windows.
Just tested Linux64 and M-C works fine there.  I also built a debug version of mozilla-beta/38 on Windows.  It consistently has this problem and on one of the attempts I ran into this assertion - not sure yet if it's related:

>	xul.dll!mozilla::ipc::Shmem::AssertInvariants() Line 317	C++
 	xul.dll!mozilla::ipc::Shmem::Size<unsigned char>() Line 173	C++
 	xul.dll!mozilla::gmp::GMPSharedMemManager::MgrAllocShmem(mozilla::gmp::GMPSharedMem::GMPMemoryClasses aClass, unsigned int aSize, mozilla::ipc::SharedMemory::SharedMemoryType aType, mozilla::ipc::Shmem * aMem) Line 44	C++
 	xul.dll!mozilla::gmp::GMPVideoEncodedFrameImpl::CreateEmptyFrame(unsigned int aSize) Line 138	C++
 	xul.dll!mozilla::WebrtcGmpVideoDecoder::Decode_g(const webrtc::EncodedImage & aInputImage, bool aMissingFrames, const webrtc::RTPFragmentationHeader * aFragmentation, const webrtc::CodecSpecificInfo * aCodecSpecificInfo, __int64 aRenderTimeMs) Line 659	C++
 	xul.dll!mozilla::runnable_args_m_5_ret<mozilla::WebrtcGmpVideoDecoder *,int (__thiscall mozilla::WebrtcGmpVideoDecoder::*)(webrtc::EncodedImage const &,bool,webrtc::RTPFragmentationHeader const *,webrtc::CodecSpecificInfo const *,__int64),webrtc::EncodedImage,bool,webrtc::RTPFragmentationHeader const *,webrtc::CodecSpecificInfo const *,__int64,int>::Run() Line 481	C++
 	xul.dll!mozilla::SyncRunnable::Run() Line 77	C++
 	xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 855	C++
 	xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 265	C++
 	xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate) Line 368	C++
 	xul.dll!MessageLoop::RunInternal() Line 234	C++
 	xul.dll!MessageLoop::RunHandler() Line 227	C++
 	xul.dll!MessageLoop::Run() Line 201	C++
 	xul.dll!nsThread::ThreadFunc(void * aArg) Line 358	C++
 	nss3.dll!_PR_NativeRunThread(void * arg) Line 397	C
 	nss3.dll!pr_root(void * arg) Line 90	C
 	[External Code]
I got this to work now by adding a return at the beginning of GMPParent::CloseIfUnused() so it never unloads OpenH264.  Perhaps we're unloading it prematurely, or counting wrong since OpenH264 is doing both encode and decode.
It's interesting. It looks like the first load returns an openh264 decoder and encoder. On the second load we return an openh264 encoder, but an Adobe CDM decoder.

This probably has something to do with my SelectPluginForAPI changes. I'll try to get this fixed today when I get to my Windows box in the office.
I think we got lucky that this ever worked.

The webrtc code is requesting a decode-video[h264] GMP, which not only matches OpenH264, but also the ClearKey and Adobe CDMs.

When the OpenH264 GMP is added back onto the plugin list after shutdown, it is added to the end of the GMPService::mPlugins array, so it's prioritised last.

We *could* make GMPParent::CanBeSharedCrossNodeIds to return |false| for CDMs, but IIRC we plan on using CDMs for non-encrypted decoding at some point.

I think the best thing to do here is to add a new GMP tag to the |APIs| line of gmpopenh264.info so that we can tell it apart.
We need a solution that works with the existing v1.3 and v1.4 plugins and that can be released with FF38 or everyone using H264 on Windows will break with FF38 releases.

Longer term we could release a v1.4.1 that changes the .info file but we are already in the middle of the v1.4 rollout.
Attachment #8599082 - Flags: review?(cpearce) → review+
Comment on attachment 8599082 [details] [diff] [review]
Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Approval Request Comment
[Feature/regressing bug #]: Adobe CDM; ClearKey CDM.
[User impact if declined]: Total WebRTC breakage on Windows.
[Describe test coverage new/current, TreeHerder]: Local testing.
[Risks and why]: Very low.
[String/UUID change made/needed]: None.

Requesting mozilla-release approval for 38; not 37.
Attachment #8599082 - Flags: approval-mozilla-release?
Attachment #8599082 - Flags: approval-mozilla-beta?
Attachment #8599082 - Flags: approval-mozilla-aurora?
[Tracking Requested - why for this release]:
I reckon this should be tracked by relman; breaking WebRTC/Firefox Hello is not something we want to let slip through the cracks.
I just made at opt build of mozilla-beta(38) on Win8 with this patch applied.  I was able to start/stop pc_test successfully 10 times, so it appears fixed.
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=9392621&repo=mozilla-inbound
Flags: needinfo?(ethanhugg)
Edwin wrote the patch and is the owner for this, but we'll look into why it bounced and try to re-land it for him before he gets in later today (in Auckland). (We = WebRTC team)
Assignee: nobody → edwin
From what I can tell, this patch tries to select only the OpenH264 plugin for WebRTC by eliminating the CDM encoders from the list.  This is working well is real situations, but these tests do not use OpenH264 unfortunately.   I believe they are using gmp-fake and if you look at dist/bin/gmp-fake/1.0/fake.info it lists APIs other than [h264] which means it gets eliminated from the list so we end up with no codecs to decode on this test.   This is my first guess at least, I haven't thought of a solution yet.
Flags: needinfo?(ethanhugg)
We can duplicate the plugin (and clean this up later), removing the decrypt API.
Assignee: edwin → rjesup
Status: NEW → ASSIGNED
didn't hit one Makefile.in
Attachment #8599496 - Attachment is obsolete: true
forgot to hg add the new files
Attachment #8599497 - Attachment is obsolete: true
Comment on attachment 8599502 [details] [diff] [review]
WIP patch to add a clone of gmp-fake that doesn't do decryption

Sorry for the firedrill, but this bug is a 38 blocker.  If people could review ASAP, everyone would appreciate it.
Attachment #8599502 - Flags: review?(ted)
Attachment #8599502 - Flags: review?(mh+mozilla)
Attachment #8599502 - Flags: review?(cpearce)
Comment on attachment 8599502 [details] [diff] [review]
WIP patch to add a clone of gmp-fake that doesn't do decryption

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

r+

I'm fine with you landing as is, but if you have time I think you should remove the stuff related to decryption from gmp-fakeopenh264, since you're not using it anyway because the .info file doesn't advertise that capability. That will make the patch smaller.

At a later date, we can remove the decoding from gmp-fake, and/or rename gmp-fake to gmp-fake-decryptor.

::: dom/media/gmp-plugin-openh264/fakeopenh264.info
@@ +1,5 @@
> +Name: fakeopenh264
> +Description: Fake GMP Plugin
> +Version: 1.0
> +APIs: encode-video[h264], decode-video[h264]
> +Libraries: dxva2.dll

The decode-video implementation doesn't use dxva.dll, so you can remove this line.

::: dom/media/gmp-plugin-openh264/gmp-fake.cpp
@@ +1,1 @@
> +/*!

Probably clearer if you renamed gmp-fake.cpp to gmp-fakeopenh264.cpp.

@@ +406,5 @@
> +      return GMPNoErr;
> +    } else if (!strcmp (aApiName, GMP_API_VIDEO_ENCODER)) {
> +      *aPluginApi = new FakeVideoEncoder (static_cast<GMPVideoHost*> (aHostAPI));
> +      return GMPNoErr;
> +    } else if (!strcmp (aApiName, GMP_API_DECRYPTOR)) {

Since your .info file doesn't have an eme-decrypt, I you can remove the GMP_API_DECRYPTOR and GMP_API_ASYNC_SHUTDOWN blocks as GMP_API_DECRYPTOR won't be hit, and GMP_API_ASYNC_SHUTDOWN is not necessary for your purposes and the real OpenH264 doesn't use it. Then you can delete gmp-test-decryptor.*, gmp-test-output-protection.h, gmp-test-storage.* from gmp-plugin-openh264/ directory, and simplify your patch. You'd then not need to include their headers too.
Attachment #8599502 - Flags: review?(cpearce) → review+
tweak MOZ_GMP_PATH for mach and gtest
Attachment #8599502 - Attachment is obsolete: true
Attachment #8599502 - Flags: review?(ted)
Attachment #8599502 - Flags: review?(mh+mozilla)
Attachment #8599556 - Attachment description: WIP patch to add a clone of gmp-fake that doesn't do decryption → add a clone of gmp-fake that doesn't do decryption
Attachment #8599556 - Flags: review?(mh+mozilla)
Comment on attachment 8599556 [details] [diff] [review]
add a clone of gmp-fake that doesn't do decryption

carry forward cpearce's r+
Attachment #8599556 - Flags: review+
Comment on attachment 8599556 [details] [diff] [review]
add a clone of gmp-fake that doesn't do decryption

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

::: dom/media/gmp-plugin-openh264/Makefile.in
@@ +5,5 @@
> +
> +INSTALL_TARGETS += FAKE_GMP_OPENH264_PLUGIN
> +FAKE_GMP_OPENH264_PLUGIN_DEST = $(DEPTH)/dist/bin/gmp-fakeopenh264/1.0
> +FAKE_GMP_OPENH264_PLUGIN_FILES = \
> +  $(SHARED_LIBRARY) \

You don't have to put SHARED_LIBRARY here if you set FINAL_TARGET = 'dist/bin/gmp-fakeopenh264/1.0' in moz.build, and remove NO_DIST_INSTALL.

@@ +7,5 @@
> +FAKE_GMP_OPENH264_PLUGIN_DEST = $(DEPTH)/dist/bin/gmp-fakeopenh264/1.0
> +FAKE_GMP_OPENH264_PLUGIN_FILES = \
> +  $(SHARED_LIBRARY) \
> +  $(srcdir)/fakeopenh264.info \
> +  $(srcdir)/fakeopenh264.voucher

\
$(NULL)

@@ +9,5 @@
> +  $(SHARED_LIBRARY) \
> +  $(srcdir)/fakeopenh264.info \
> +  $(srcdir)/fakeopenh264.voucher
> +
> +include $(topsrcdir)/config/rules.mk

no need for this include.

::: dom/media/gmp-plugin-openh264/gmp-fake.cpp
@@ +1,1 @@
> +/*!

Don't copy the files, since they're identical, just use the ones in gmp-plugin. SOURCES can contain relative paths.
(and if we really wanted copies, they should be indicated as such in the patch; use hg cp)

::: dom/media/gmp-plugin-openh264/moz.build
@@ +22,5 @@
> +NO_VISIBILITY_FLAGS = True
> +# Don't use STL wrappers; this isn't Gecko code
> +DISABLE_STL_WRAPPING = True
> +
> +FAIL_ON_WARNINGS = True

This file should be indicated as a copy of dom/media/gmp-plugin/moz.build.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +675,5 @@
>          gtest_env = {b'GTEST_FILTER': gtest_filter}
>  
>          xre_path = os.path.join(self.topobjdir, "dist", "bin")
>          gtest_env["MOZ_XRE_DIR"] = xre_path
> +        gtest_env["MOZ_GMP_PATH"] = os.path.join(xre_path, "gmp-fake", "1.0") + os.pathsep + os.path.join(xre_path, "gmp-fakeopenh264", "1.0")

os.pathsep.join(os.path.join(xre_path, p, '1.0') for p in ('gmp-fake', 'gmp-fakeopenh264'))

::: testing/gtest/rungtests.py
@@ +66,5 @@
>          Add environment variables likely to be used across all platforms, including remote systems.
>          """
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] = os.path.join(self.xre_path, "gmp-fake", "1.0") + os.pathsep + os.path.join(self.xre_path, "gmp-fakeopenh264", "1.0")

likewise

::: testing/mochitest/mach_commands.py
@@ +469,5 @@
> +                options.gmp_path += os.pathsep
> +                options.gmp_path = os.path.join(
> +                    os.path.dirname(bin_path),
> +                    'gmp-fakeopenh264',
> +                    '1.0')

This entire section could use some factoring, like:

gmp_modules = (
    ('gmp-fake', '1.0'),
    ('gmp-clearkey', '0.1'),
    ('gmp-fakeopenh264', '1.0'),
)

options.gmp_path = os.pathsep.join(os.path.join(os.path.dirname(bin_path), *p) for p in gmp_modules)
Attachment #8599556 - Flags: review?(mh+mozilla) → review-
incorporates glandium's and cpearce's reviews
Attachment #8599616 - Flags: review?(mh+mozilla)
Attachment #8599556 - Attachment is obsolete: true
Attachment #8599616 - Flags: review+
Comment on attachment 8599616 [details] [diff] [review]
WIP patch to add a clone of gmp-fake that doesn't do decryption

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

::: dom/media/gmp-plugin-openh264/moz.build
@@ +1,1 @@
> +# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-

Still not marked as a copy in the patch. Please use hg cp.

@@ +5,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# largely a copy of dom/media/gmp-fake/moz.build
> +
> +FINAL_TARGET='dist/bin/gmp-fakeopenh264/1.0'

spaces around =

@@ +7,5 @@
> +# largely a copy of dom/media/gmp-fake/moz.build
> +
> +FINAL_TARGET='dist/bin/gmp-fakeopenh264/1.0'
> +SOURCES += [
> +        '../gmp-plugin/gmp-fake.cpp',

4-space indents

::: testing/gtest/rungtests.py
@@ +66,5 @@
>          Add environment variables likely to be used across all platforms, including remote systems.
>          """
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] = os.pathsep.join(os.path.join(self.xre_path, p, "1.0") for p in ('gmp-fake', 'gmp-fakeopenh264'))

might be better to add some carriage returns, like:

os.pathset.join(
  os.path.join(...)
  for ...
)

::: testing/mochitest/mach_commands.py
@@ +458,5 @@
>                  # shipped in the binary
>                  bin_path = self.get_binary_path()
> +                gmp_modules = (
> +                    ('gmp-fake', '1.0'),
> +                    ('gmp-clearkey', '1.0'),

0.1
Attachment #8599616 - Flags: review?(mh+mozilla) → feedback+
Comment on attachment 8599631 [details] [diff] [review]
Patch2: Add a clone of gmp-fake that doesn't do decryption

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

::: dom/media/gmp-plugin-openh264/Makefile.in
@@ +7,5 @@
> +FAKE_GMP_OPENH264_PLUGIN_DEST = $(DEPTH)/dist/bin/gmp-fakeopenh264/1.0
> +FAKE_GMP_OPENH264_PLUGIN_FILES = \
> +  $(srcdir)/fakeopenh264.info \
> +  $(srcdir)/fakeopenh264.voucher \
> +  $(NULL)

As a followup, you can do the same cleanup in dom/media/gmp-plugin.

::: testing/gtest/rungtests.py
@@ +67,5 @@
>          """
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] =
> +            os.pathsep.join(

that needs to be on the same line as the =, otherwise, it's a SyntaxError.

@@ +68,5 @@
>          env["MOZILLA_FIVE_HOME"] = self.xre_path
>          env["MOZ_XRE_DIR"] = self.xre_path
> +        env["MOZ_GMP_PATH"] =
> +            os.pathsep.join(
> +                os.path.join(self.xre_path, p, "1.0")

and those dedented.

@@ +70,5 @@
> +        env["MOZ_GMP_PATH"] =
> +            os.pathsep.join(
> +                os.path.join(self.xre_path, p, "1.0")
> +                for p in ('gmp-fake', 'gmp-fakeopenh264')
> +            )

The same line wrapping should be done in mach_commands.py
Attachment #8599631 - Flags: review?(mh+mozilla) → review+
Attachment #8599631 - Attachment is obsolete: true
Attachment #8599082 - Attachment is obsolete: true
Attachment #8599082 - Flags: approval-mozilla-release?
Attachment #8599082 - Flags: approval-mozilla-beta?
Attachment #8599082 - Flags: approval-mozilla-aurora?
Attachment #8599631 - Attachment is obsolete: false
Attachment #8599082 - Attachment is obsolete: false
Attachment #8599616 - Attachment is obsolete: true
Attachment #8599631 - Attachment description: WIP patch to add a clone of gmp-fake that doesn't do decryption → Add a clone of gmp-fake that doesn't do decryption
Attachment #8599631 - Flags: review+
Final "what the hell, waste some resources" try failed because   Bug 1155939 landed just before I did my try and breaks when there are two plugins that can decode video

edwin, cpearce: can you resolve this and land it ASAP on inbound?  We desperately need to get this in and uplifted to 38.
Flags: needinfo?(edwin)
Flags: needinfo?(cpearce)
Problem is as jesup pointed out; now we have two fake plugins. The easiest fix is to make the GeckoMediaPlugins.RemoveAndDeleteForcedInUse gtest ask for a video decoder that also supports decrypting, that way we can narrow the test down to only try to use one specific fake gmp type.

This patch should only be required on m-i, not aurora or beta.
Flags: needinfo?(cpearce)
Attachment #8599759 - Flags: review?(edwin)
Flags: needinfo?(edwin)
Attachment #8599759 - Flags: review?(edwin) → review+
Attachment #8599082 - Attachment description: 1159300.patch → Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback
Attachment #8599103 - Attachment description: 1159300-aurora.patch → [Aurora39] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback
Attachment #8599687 - Attachment description: Don't use decrypting Gecko Media Plugins for non-encrypted playback (Fx38 version)- → [Beta39] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback
Attachment #8599687 - Flags: review+
Attachment #8599631 - Attachment description: Add a clone of gmp-fake that doesn't do decryption → Patch2: Add a clone of gmp-fake that doesn't do decryption
Attachment #8599686 - Attachment description: Add a clone of gmp-fake that doesn't do decryption (fx38 version) → [Beta38] Patch 2: Add a clone of gmp-fake that doesn't do decryption (fx38 version)
Attachment #8599759 - Attachment description: Patch3: Fix GMP gtest → [Uplift Not Required] Patch3: Fix GMP gtest
Attachment #8599687 - Attachment description: [Beta39] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback → [Beta38] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback
Comment on attachment 8599687 [details] [diff] [review]
[Beta38] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Approval Request Comment
[Feature/regressing bug #]: Something in EME regressed this
[User impact if declined]: Users won't be able to make a second WebRTC call in the same Firefox instance using OpenH264 GMP.
[Describe test coverage new/current, TreeHerder]: We have plenty of EME tests, so this won't break EME. I don't think the coverage of the OpenH264 use cases is great, but our test suites can't test OpenH264 directly, so it's hard.
[Risks and why]: Pretty low; basically makes us share plugin instances across plugin users less.
[String/UUID change made/needed]: None.
Attachment #8599687 - Flags: approval-mozilla-beta?
Comment on attachment 8599103 [details] [diff] [review]
[Aurora39] Patch 1: Don't use decrypting Gecko Media Plugins for non-encrypted playback

Approval Request Comment
[Feature/regressing bug #]: Something in EME regressed this
[User impact if declined]: Users won't be able to make a second WebRTC call in the same Firefox instance using OpenH264 GMP.
[Describe test coverage new/current, TreeHerder]: We have plenty of EME tests, so this won't break EME. I don't think the coverage of the OpenH264 use cases is great, but our test suites can't test OpenH264 directly, so it's hard.
[Risks and why]: Pretty low; basically makes us share plugin instances across plugin users less.
[String/UUID change made/needed]: None.
Attachment #8599103 - Flags: approval-mozilla-aurora?
Attachment #8599687 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Comment on attachment 8599686 [details] [diff] [review]
[Beta38] Patch 2: Add a clone of gmp-fake that doesn't do decryption (fx38 version)

Approval Request Comment
[Feature/regressing bug #]: Something in EME regressed this
[User impact if declined]: This makes the tests pass when patch 1 is applied.
[Describe test coverage new/current, TreeHerder]: We have enough tests to fail without this fix. ;) Patch 1 makes EME plugins less sharable, and that means the tests targeted at our "fake" OpenH264, which had a dummy EME decryptor in it, wasn't available to the tests. This moved the OpenH264 fake out into a separate test plugin. So this patch only affects tests.
[Risks and why]: Pretty low; this only affects tests.
[String/UUID change made/needed]: None.
Attachment #8599686 - Flags: approval-mozilla-beta?
Attachment #8599686 - Flags: approval-mozilla-aurora?
Attachment #8599686 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Attachment #8599103 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8599686 [details] [diff] [review]
[Beta38] Patch 2: Add a clone of gmp-fake that doesn't do decryption (fx38 version)

38 is an ESR, we want a clean testsuite. Taking it.
Should be in 38 RC1.
Attachment #8599686 - Flags: approval-mozilla-release?
Attachment #8599686 - Flags: approval-mozilla-release+
Attachment #8599686 - Flags: approval-mozilla-aurora?
Attachment #8599686 - Flags: approval-mozilla-aurora+
Attachment #8599687 - Flags: approval-mozilla-release? → approval-mozilla-release+
https://hg.mozilla.org/mozilla-central/rev/7792f4778da8
https://hg.mozilla.org/mozilla-central/rev/491bcd3dba9f
https://hg.mozilla.org/mozilla-central/rev/a7da86419656
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Tested with local build of Opt build M-C on Win8.  Was able to start/stop pc_test.html every time.  Also tested with talky.io and media.navigator.video.preferred_codec=126, and using CiscoSpark.com.  All worked fine with e10s disabled.
Tracking for 39 and 40 while we verify the fix.
Flags: needinfo?(florin.mezei)
Flags: qe-verify+
Comment 42 says that this only affects tests, so I'm not sure what kind of manual verification is needed here. Jen or Liz, could you provide some details?
Flags: needinfo?(florin.mezei)
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #51)
> Comment 42 says that this only affects tests, so I'm not sure what kind of
> manual verification is needed here. Jen or Liz, could you provide some
> details?

Patch 2 (referred to by comment 42) is in fact something that only affects mochitests.
Sorry, my bad, I missed that it only affected tests. Removed tracking for 39 and 40.
(In reply to Randell Jesup [:jesup] from comment #52)
> Patch 2 (referred to by comment 42) is in fact something that only affects
> mochitests.

Patch 1 however is quite testable; see description
Verified as fixed using the following environment:

FF 38 ESR
Build Id: 20150505103531
OS: Win 8.1 x86
Removing qe-verify flag as verification on Firefox 38 Beta should suffice.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.