Closed Bug 1028962 Opened 6 years ago Closed 6 years ago

Gecko Media Plugin doesn't work on Aurora Release

Categories

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

32 Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox30 --- unaffected
firefox31 --- unaffected
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: ehugg, Assigned: ehugg)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

The GMP plugin doesn't work on Aurora Release when pulled from tbpl (including Try).

It does however work when built locally.
Aurora Debug works from tbpl.
Nightly works in all cases.

The nspr logs show this:
ConfigureSendMediaCodec SetSendCodec Failed 0

and putting some debugging in GMPChild.cpp::LoadPluginLibrary showed that it is never called in this case so it's not even trying to load the plugin.
ViEEncoder::SetEncoder() was failing on this build because the frame rate was set to zero.  In this code in VideoConduit:
http://mxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#1292
It appears that the codecInfo->maxFramerate is zero so the outgoing cinst is whatever was on the stack earlier when video_codec was declared on the stack here:
http://mxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#548
Evidently this is zero in this particular build and some random number when I built other times (173 on my local machine).

So, the easiest fix would be to set cinst.maxFramerate to some default if the passed-in maxFramerate is zero.

Jesup, do you have a suggestion for the right default value here or do you see a larger fix to be made?
Flags: needinfo?(rjesup)
Try run where I hardcoded the maxFramerate which now works - 
https://tbpl.mozilla.org/?tree=Try&rev=65355c837289

I'm not sure it matters yet what the maxFramerate is set to as long as it's not zero.
Assignee: nobody → ethanhugg
Status: NEW → ASSIGNED
Comment on attachment 8444818 [details] [diff] [review]
Fix for setting maxFramerate with Gecko Media Plugins.

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

This is what I suggest to keep it simple so it can be uplifted.  The default max framerate I got from the webrtc tests here - 
http://mxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/video_engine/test/auto_test/source/vie_autotest_custom_call.cc#40
Attachment #8444818 - Flags: review?(rjesup)
Try on Aurora Release with this patch which works:
https://tbpl.mozilla.org/?tree=Try&rev=3d64a3cfcdb2
Blocks: OpenH264
Comment on attachment 8444818 [details] [diff] [review]
Fix for setting maxFramerate with Gecko Media Plugins.

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

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ +555,5 @@
>    int error = 0; //webrtc engine errors
>    webrtc::VideoCodec  video_codec;
>    std::string payloadName;
>  
> +  memset(&video_codec, 0, sizeof(webrtc::VideoCodec));

sizeof(video_codec) - good practice to avoid "I changed/subclassed the struct" bugs.
Attachment #8444818 - Flags: review?(rjesup) → review+
Attachment #8444818 - Attachment is obsolete: true
Comment on attachment 8445900 [details] [diff] [review]
Fix for setting maxFramerate with Gecko Media Plugins.


Changed sizeof and rebased on latest.
Bringing forward r+ from jesup.
Attachment #8445900 - Flags: review+
Flags: needinfo?(rjesup)
Keywords: leave-open
On Central, asking for uplift
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8445900 [details] [diff] [review]
Fix for setting maxFramerate with Gecko Media Plugins.

Approval Request Comment
[Feature/regressing bug #]: Basic OpenH264 support (which is preffed off)

[User impact if declined]: breaks ability to test OpenH264 on Aurora (OpenH264 is preffed off; expected to pref on in 33).  In theory the patch could help VP8, but apparently it doesn't care about the mex-framerate setting.

[Describe test coverage new/current, TBPL]: OpenH264 is preffed off; tests will be landing as part of 33.  Testing OpenH264 is tough since it's a plugin and TBPL isn't set up well for testing them.

[Risks and why]: no risk; removes an instance of uninitialized memory.

[String/UUID change made/needed]: none
Attachment #8445900 - Flags: approval-mozilla-aurora?
Comment on attachment 8445900 [details] [diff] [review]
Fix for setting maxFramerate with Gecko Media Plugins.

Aurora approval granted.
Attachment #8445900 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.