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

Append element to dom for LocalTrack setProcessor #981

Closed
Kanary159357 opened this issue Jan 2, 2024 · 4 comments · Fixed by #984
Closed

Append element to dom for LocalTrack setProcessor #981

Kanary159357 opened this issue Jan 2, 2024 · 4 comments · Fixed by #984

Comments

@Kanary159357
Copy link

Kanary159357 commented Jan 2, 2024

Describe the problem

My team is currently working on noise reduction during streaming with livekit with love.
I add NoiseReduction logic to track with setProcessor method in local track.
Noise reduction works well, but there is one problem.

After adding setProcessor, the sound of track get out to speaker.

It's because there are audio playing element on processorElement.

In setProcessor method, there are logic that if there is no processorElement, get new element by document.createElement.

     this.processorElement = this.processorElement ?? document.createElement(this.kind);
      this.processorElement.muted = true;

      attachToElement(this._mediaStreamTrack, this.processorElement);
      this.processorElement
        .play()
        .catch((error) => log.error('failed to play processor element', { error }));

But this element is not attached to DOM, so can not control it by document.getElement~~.
Also, processorElement is protected, so cannot control it from local track.

So audio is kept played

Describe the proposed solution

I think there are several ways to solve it. But don't know what would this project prefer.

  1. Add this processorElement to DOM with certain ID in order to get element and control it with document API

Alternatives considered

  1. change processorElement access modifier from protected to public like attachedElement

Importance

I cannot use LiveKit without it

Additional Information

No response

@lukasIO
Copy link
Contributor

lukasIO commented Jan 2, 2024

What would you like to control about the dom element?
The element is automatically muted, so I'm not sure what you mean by

the sound of track get out to speaker.

What's the unexpected behaviour here?

@Kanary159357
Copy link
Author

Kanary159357 commented Jan 3, 2024

What would you like to control about the dom element? The element is automatically muted, so I'm not sure what you mean by

the sound of track get out to speaker.

What's the unexpected behaviour here?

The element is not muted actaully.

 this.processorElement.muted = true;

First initialized with muted, but

 attachToElement(this._mediaStreamTrack, this.processorElement);

attachToElement method set this element muted property as true again.
https://github.com/livekit/client-sdk-js/blob/75b0abd92b5eaa9af2c3a729e81ae981111a4e79/src/room/track/Track.ts#L308C3-L308C3

  element.muted = mediaStream.getAudioTracks().length === 0;

So muted property set to true again when there is audioTrack

If it is intended to mute the processElement. I think we should change the order of codes(attachElement first, then set element muted)

@lukasIO
Copy link
Contributor

lukasIO commented Jan 3, 2024

If the processor element is muted properly, then this would solve your use case ?

@Kanary159357
Copy link
Author

If the processor element is muted properly, then this would solve your use case ?

Yes!
The problem was that processor audio is playing so if muted, no problem

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 a pull request may close this issue.

2 participants