Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MediaStreamAudioDestinationNode, MediaStreamAudioSourceNode, MediaStreamTrackAudioSourceNode #27143

Merged
merged 10 commits into from Jul 3, 2020

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Jul 2, 2020

Progress in #26097

This is a draft since we need the data channels stuff to land first

(also I need to make sure we're passing WPT)

@Manishearth Manishearth requested a review from ferjm Jul 2, 2020
@highfive
Copy link

highfive commented Jul 2, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/mediastreamaudiodestinationnode.rs, components/script/dom/webidls/MediaStreamAudioSourceNode.webidl, components/script/dom/mediastreamaudiosourcenode.rs, components/script/dom/mediastreamtrackaudiosourcenode.rs, components/script/dom/mod.rs and 3 more
  • @KiChjang: components/script/dom/mediastreamaudiodestinationnode.rs, components/script/dom/webidls/MediaStreamAudioSourceNode.webidl, components/script/dom/mediastreamaudiosourcenode.rs, components/script/dom/mediastreamtrackaudiosourcenode.rs, components/script/dom/mod.rs and 3 more

@highfive
Copy link

highfive commented Jul 2, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!

ferjm
ferjm approved these changes Jul 2, 2020
Copy link
Member

@ferjm ferjm left a comment

Looks good! Only a few minor issues to fix.

The data channels PR landed a couple of days ago. So this should be good to land after a rebase

&context.upcast(),
node_options,
1, // inputs
1, // outputs
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a destination node, shouldn't this be 0 outputs?

[Exposed=Window]
interface MediaStreamTrackAudioSourceNode : AudioNode {
[Throws] constructor (AudioContext context, MediaStreamTrackAudioSourceOptions options);
};
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newline at end of file

interface MediaStreamAudioDestinationNode : AudioNode {
[Throws] constructor (AudioContext context, optional AudioNodeOptions options = {});
readonly attribute MediaStream stream;
};
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newline at end of file

AudioNodeInit::MediaStreamSourceNode(track),
&context.upcast(),
node_options,
1, // inputs
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 inputs?

AudioNodeInit::MediaStreamSourceNode(track),
&context.upcast(),
node_options,
1, // inputs
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, as a source node, this should probably have 0 inputs.

count: 2,
mode: ChannelCountMode::Max,
interpretation: ChannelInterpretation::Speakers,
};
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the default values, so we can do Default::default() here.

count: 2,
mode: ChannelCountMode::Max,
interpretation: ChannelInterpretation::Speakers,
};
Copy link
Member

@ferjm ferjm Jul 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Default::default() here, please.

@Manishearth Manishearth force-pushed the streamnodes branch 3 times, most recently from d715608 to 70411ac Compare Jul 2, 2020
@Manishearth Manishearth marked this pull request as ready for review Jul 2, 2020
@Manishearth
Copy link
Member Author

Manishearth commented Jul 2, 2020

@bors-servo r=ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

📌 Commit 70ae248 has been approved by ferjm

bors-servo added a commit that referenced this issue Jul 2, 2020
Implement MediaStreamAudioDestinationNode, MediaStreamAudioSourceNode, MediaStreamTrackAudioSourceNode

Progress in #26097

This is a draft since we need the data channels stuff to land first

(also I need to make sure we're passing WPT)
@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

Testing commit 70ae248 with merge 2223215...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jul 2, 2020

./tests/html/media_stream_audio_nodes_loopback.html:12: no newline at EOF

Sigh.

@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

Testing commit 06bacf3 with merge 8537f76...

bors-servo added a commit that referenced this issue Jul 2, 2020
Implement MediaStreamAudioDestinationNode, MediaStreamAudioSourceNode, MediaStreamTrackAudioSourceNode

Progress in #26097

This is a draft since we need the data channels stuff to land first

(also I need to make sure we're passing WPT)
@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

💔 Test failed - status-taskcluster

@Manishearth
Copy link
Member Author

Manishearth commented Jul 2, 2020

@bors retry

  • network error on rustfmt

@Manishearth
Copy link
Member Author

Manishearth commented Jul 2, 2020

@bors-servo retry

spending too much time in rust land

@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

Testing commit 06bacf3 with merge 854edb4...

bors-servo added a commit that referenced this issue Jul 2, 2020
Implement MediaStreamAudioDestinationNode, MediaStreamAudioSourceNode, MediaStreamTrackAudioSourceNode

Progress in #26097

This is a draft since we need the data channels stuff to land first

(also I need to make sure we're passing WPT)
@bors-servo
Copy link
Contributor

bors-servo commented Jul 2, 2020

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Member

CYBAI commented Jul 3, 2020

  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: operation createMediaStreamSource(MediaStream)
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: operation createMediaStreamTrackSource(MediaStreamTrack)
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: operation createMediaStreamDestination()
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: context must inherit property "createMediaStreamSource(MediaStream)" with the proper type
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: calling createMediaStreamSource(MediaStream) on context with too few arguments must throw TypeError
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: context must inherit property "createMediaStreamTrackSource(MediaStreamTrack)" with the proper type
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: calling createMediaStreamTrackSource(MediaStreamTrack) on context with too few arguments must throw TypeError
  ▶ Unexpected subtest result in /webaudio/idlharness.https.window.html:
  └ PASS [expected FAIL] AudioContext interface: context must inherit property "createMediaStreamDestination()" with the proper type
  ▶ TIMEOUT [expected OK] /webaudio/the-audio-api/the-mediastreamaudiosourcenode-interface/mediastreamaudiosourcenode-routing.html
  │ 
  │ error: XDG_RUNTIME_DIR not set in the environment.
  │ libEGL warning: No hardware driver found, falling back to software rendering
  │ 0:00:00.497616481 10581 0x7f798c013d00 ERROR             jackclient gstjackaudioclient.c:35:jack_log_error: Cannot connect to server socket err = No such file or directory
  │ 0:00:00.497662445 10581 0x7f798c013d00 ERROR             jackclient gstjackaudioclient.c:35:jack_log_error: Cannot connect to server request channel
  │ 0:00:00.499792390 10581 0x7f798c013d00 ERROR             jackclient gstjackaudioclient.c:35:jack_log_error: jack server is not running or cannot be started
  │ 0:00:00.500007253 10581 0x7f798c013d00 ERROR             jackclient gstjackaudioclient.c:35:jack_log_error: JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
  │ 0:00:00.500015279 10581 0x7f798c013d00 ERROR             jackclient gstjackaudioclient.c:35:jack_log_error: JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for -1, skipping unlock
  │ 0:00:00.500020264 10581 0x7f798c013d00 WARN              jackclient gstjackaudioclient.c:381:gst_jack_audio_get_connection: could not create connection
  │ 0:00:00.500068384 10581 0x7f798c013d00 WARN                jacksink gstjackaudiosink.c:355:gst_jack_ring_buffer_open_device:<autoaudiosink0-actual-sink-jackaudio> error: Jack server not found
  │ 0:00:00.500074697 10581 0x7f798c013d00 WARN                jacksink gstjackaudiosink.c:355:gst_jack_ring_buffer_open_device:<autoaudiosink0-actual-sink-jackaudio> error: Cannot connect to the Jack server (status 17)
  │ 0:00:00.500697643 10581 0x7f798c013d00 WARN                 default oss4-property-probe.c:302:gst_oss4_property_probe_get_values:<autoaudiosink0-actual-sink-oss4> Can't open file descriptor to probe available devices: No such file or directory
  │ 0:00:00.500721718 10581 0x7f798c013d00 WARN                oss4sink oss4-sink.c:513:gst_oss4_sink_open:<autoaudiosink0-actual-sink-oss4> error: Could not open audio device for playback.
  │ 0:00:00.500727445 10581 0x7f798c013d00 WARN                oss4sink oss4-sink.c:513:gst_oss4_sink_open:<autoaudiosink0-actual-sink-oss4> error: system error: No such file or directory
  │ ALSA lib confmisc.c:767:(parse_card) cannot find card '0'
  │ ALSA lib conf.c:4568:(_snd_config_evaluate) function snd_func_card_driver returned error: No such file or directory
  │ ALSA lib confmisc.c:392:(snd_func_concat) error evaluating strings
  │ ALSA lib conf.c:4568:(_snd_config_evaluate) function snd_func_concat returned error: No such file or directory
  │ ALSA lib confmisc.c:1246:(snd_func_refer) error evaluating name
  │ ALSA lib conf.c:4568:(_snd_config_evaluate) function snd_func_refer returned error: No such file or directory
  │ ALSA lib conf.c:5047:(snd_config_expand) Evaluate error: No such file or directory
  │ ALSA lib pcm.c:2564:(snd_pcm_open_noupdate) Unknown PCM default
  │ AL lib: (EE) ALCplaybackAlsa_open: Could not open playback device 'default': No such file or directory
  │ 0:00:00.511310244 10581 0x7f798c013d00 WARN                  openal gstopenalsink.c:634:gst_openal_sink_open:<autoaudiosink0-actual-sink-openal> error: Could not open device.
  │ 0:00:00.511323336 10581 0x7f798c013d00 WARN                  openal gstopenalsink.c:634:gst_openal_sink_open:<autoaudiosink0-actual-sink-openal> error: ALC error: Out of Memory
  │ 0:00:00.511859379 10581 0x7f798c013d00 WARN                     oss gstosssink.c:398:gst_oss_sink_open:<autoaudiosink0-actual-sink-oss> error: Could not open audio device for playback.
  │ 0:00:00.511868946 10581 0x7f798c013d00 WARN                     oss gstosssink.c:398:gst_oss_sink_open:<autoaudiosink0-actual-sink-oss> error: system error: No such file or directory
  │ 0:00:00.512462260 10581 0x7f798c006d40 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc0:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  │ 0:00:00.513797687 10581 0x7f798c046850 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc1:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  └ 0:00:00.514872193 10581 0x7f79f811fde0 FIXME                default gstutils.c:3980:gst_pad_create_stream_id_internal:<appsrc2:src> Creating random stream-id, consider implementing a deterministic way of creating a stream-id
  ▶ Unexpected subtest result in /webaudio/the-audio-api/the-mediastreamaudiosourcenode-interface/mediastreamaudiosourcenode-routing.html:
  │ TIMEOUT [expected FAIL] MediaStreamAudioSourceNode captures the right track.
  └   → Test timed out
  ▶ Unexpected subtest result in /webaudio/the-audio-api/the-audionode-interface/audionode-connect-method-chaining.html:
  └ PASS [expected FAIL] Executing "media-group"

@Manishearth
Copy link
Member Author

Manishearth commented Jul 3, 2020

@bors-servo r=ferjm

Missed some changes in the rebasing.

The routing timeout is due to a bug in the analyzer node, not a bug in the media node

@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2020

📌 Commit 3b3e2e0 has been approved by ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2020

Testing commit 3b3e2e0 with merge 4504eeb...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2020

☀️ Test successful - status-taskcluster
Approved by: ferjm
Pushing 4504eeb to master...

@bors-servo bors-servo merged commit 4504eeb into servo:master Jul 3, 2020
2 checks passed
@bors-servo bors-servo mentioned this pull request Jul 3, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants