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

feat: added seek & rate to SpeakerAudioDestination #504

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

calinalexandru
Copy link

Hello,

In my application I need to be able to adjust seek & rate of the Audio element, and I don't believe there's a reason for them not to be here.
Also, I set privAudio = new Audio() from the constructor to prevent unnecessary undefined check.
Went ahead and removed some of those checks, although, more expressions can be simplified in the class.

@ghost
Copy link

ghost commented Mar 18, 2022

CLA assistant check
All CLA requirements met.

@glharper
Copy link
Member

@calinalexandru Thanks for submitting this! The code looks good, I'll run this through some manual tests and give the team and chance to look at it.

@glharper glharper requested a review from yulin-li March 22, 2022 15:09
Copy link
Contributor

@yulin-li yulin-li left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! The new added api looks good to me. Added one comment

@@ -52,6 +52,7 @@ export class SpeakerAudioDestination implements IAudioDestination, IPlayer {
this.privId = audioDestinationId ? audioDestinationId : createNoDashGuid();
this.privIsPaused = false;
this.privIsClosed = false;
this.privAudio = new Audio();
Copy link
Contributor

Choose a reason for hiding this comment

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

The undefined value of privaAudio means the playback is not supported (e.g. unsupported format, see line 134). This would break some current logic. So pls revert.

Copy link
Author

Choose a reason for hiding this comment

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

@yulin-li can you please elaborate your response? From what I can see calling method format will re-initialize privAudio so it doesn't matter if I also set it constructor. I'm only trying to understand if there's a better approach than re-adding undefined checks (following null object pattern). Thanks.

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.

3 participants