GMP fake plugin test should test that the GMP plugin actually launches and does something

RESOLVED FIXED in mozilla33

Status

()

Core
WebRTC: Audio/Video
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Benjamin Smedberg, Assigned: jesup)

Tracking

(Blocks: 1 bug)

unspecified
mozilla33
All
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
I added a test of the fake GMP plugin in bug 1037125. This test currently just waits for webrtc to claim a connection is established, and doesn't actually check that we are receiving decoded frames. It should do more checking.
Flags: needinfo?(mreavy)
Jesup's already in this code, so he wants to finish this off.
Assignee: nobody → rjesup
Flags: needinfo?(mreavy)
(Assignee)

Comment 2

4 years ago
Created attachment 8459200 [details] [diff] [review]
Basic webrtc smoketests for GMP fake H.264 plugin
(Assignee)

Updated

4 years ago
Attachment #8459200 - Flags: review?(drno)
(Assignee)

Comment 3

4 years ago
Created attachment 8459201 [details] [diff] [review]
Basic webrtc smoketests for GMP fake H.264 plugin
(Assignee)

Updated

4 years ago
Attachment #8459200 - Attachment is obsolete: true
Attachment #8459200 - Flags: review?(drno)
(Assignee)

Updated

4 years ago
Attachment #8459201 - Flags: review?(drno)
Comment on attachment 8459201 [details] [diff] [review]
Basic webrtc smoketests for GMP fake H.264 plugin

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

Its ok to land this like it is, although I would prefer to have my comments for pc.js addressed. Depending on the time pressure we can also address them later.
Let me know if I should make the changes and carry this over the finish line for you.

::: dom/media/tests/mochitest/pc.js
@@ +359,5 @@
>  
> +// Also remove mode 0 if it's offered
> +// Note, we don't bother removing the fmtp lines, which makes a good test
> +// for some SDP parsing issues.
> +function removeVP8(sdp) {

Wondering if should make this a generic helpers which is able to remove any given codec number from a given SDP.

@@ +365,5 @@
> +  updated_sdp = updated_sdp.replace("RTP/SAVPF 120 126 97\r\n","RTP/SAVPF 126 97\r\n");
> +  updated_sdp = updated_sdp.replace("RTP/SAVPF 120 126\r\n","RTP/SAVPF 126\r\n");
> +  updated_sdp = updated_sdp.replace("a=rtcp-fb:120 nack\r\n","");
> +  updated_sdp = updated_sdp.replace("a=rtcp-fb:120 nack pli\r\n","");
> +  updated_sdp = updated_sdp.replace("a=rtcp-fb:120 ccm fir\r\n","");

Can we remove all of them with one statement?

@@ +1355,5 @@
>   *        Description for the peer connection instance
>   * @param {object} configuration
>   *        Configuration for the peer connection instance
>   */
> +function PeerConnectionWrapper(label, configuration, h264) {

I would really prefer to have the H264 things as a PCW function. The list of things we might want to modify and/or check can easily grow fast otherwise.

::: dom/media/tests/mochitest/test_peerConnection_basicH264Video.html
@@ +20,5 @@
> +  var test;
> +  runNetworkTest(function (options) {
> +    options = options || { };
> +    options.h264 = true;
> +    test = new PeerConnectionTest(options);

A function call instead of setting it via the options would be preferred from my perspective (see comment pc.js).
Attachment #8459201 - Flags: review?(drno) → review+
(Assignee)

Comment 5

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce030325f21c
Target Milestone: --- → mozilla33
https://hg.mozilla.org/mozilla-central/rev/ce030325f21c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.