971830 - Utilize get(Audio|Video)Tracks() from 834835 in mochitest

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Nils Ohlmeier [:drno]]

Utilize getAudioTracks() and getVideoTracks() from bug 834835 in mochitest to replace the hardwired values in pc.js.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: properly_name_remote_audio_video_streams.patch

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8374974 [details] [diff] [review]
properly_name_remote_audio_video_streams.patch

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

Looks fine to me. Please fix the nits and you have me r+. Did you send it already to try? I wonder if we need other fixes for checks regarding the new 'audio' part in the type string. In the future you might want to add the try build request as comment to the bug.

::: dom/media/tests/mochitest/pc.js
@@ +1105,4 @@
>  
> +    var type = '';
> +    if (event.stream.getAudioTracks().length > 0) {
> +        type = 'audio';

nit: 2 blanks instead of 4 please.

Comment 3

[Nils Ohlmeier [:drno]]

Attachment: properly_name_remote_audio_video_streams.patch

Addressed the nits. Carrying forward r+=hskupin

Comment 4

[Nils Ohlmeier [:drno]]

Try server run: https://tbpl.mozilla.org/?tree=Try&rev=2249d7b6dda3

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Nils Ohlmeier [:drno] from comment #4)
> Try server run: https://tbpl.mozilla.org/?tree=Try&rev=2249d7b6dda3

This build caused a failure on Windows 8 debug in the basic Audio/Video peerconnection test. Can we be sure it is not a problem by this patch?

https://tbpl.mozilla.org/php/getParsedLog.php?id=34716571&tree=Try

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7173c66302c3

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7173c66302c3