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

Safari Channel Order Helper #10

Closed
danrossi opened this issue Jul 30, 2016 · 5 comments
Closed

Safari Channel Order Helper #10

danrossi opened this issue Jul 30, 2016 · 5 comments

Comments

@danrossi
Copy link
Contributor

I'm not sure if this is correct but I believe something like this could help dynamically fix the safari channel order from the original channel map config.

This Is what I have implemented according to the documentation.

If this works would this be of interest to add as a global static util ?

OmnitoneUtils.channelMapSafari = function (channels) {
    channels.splice(0, 0, channels.splice(2, 1)[0]);
    return channels;
};
@hoch
Copy link
Member

hoch commented Aug 1, 2016

Thanks for your contribution, but this feature is already in the code:
https://github.com/GoogleChrome/omnitone#foadecoder
https://github.com/GoogleChrome/omnitone#foarouter

FYI, this is much faster than splicing array.

@hoch hoch closed this as completed Aug 1, 2016
@danrossi
Copy link
Contributor Author

danrossi commented Aug 2, 2016

Are you saying it is ordering internally already for Safari sorry ? Or externally it is required to manually reorder for Safari depending what the custom channel order is ?

It's confusing what is required in regards to that config that is all.

I've implemented it here so the user doesn't have to do anything at all to treat it differently for Safari. I've tested Safari now and it seems to be working with the cors proxy of course and an mp4.

The demo video from the site seems to work across browsers but I hear phasing because the source is very noisy. So the noise floor is doubling which increases it's gain, then when rotating I can hear the phasing. It'a possibly self noise from a bad mic or preamp noise from low grade equipment.

https://github.com/danrossi/three-vr-omnitone/blob/master/src/OmniToneAudio.js#L109

@hoch
Copy link
Member

hoch commented Aug 2, 2016

@danrossi You can do whatever you want in your package and I appreciate your effort on that. It is really nice to have a package that connects Omnitone and Three.js. However, I want to keep the channel rerouting option flexible within Omnitone. I don't want to make things special only for Safari.

Regarding the sound quality, I suggest you to file another entry for that. It is not relevant to this issue.

@danrossi
Copy link
Contributor Author

danrossi commented Aug 2, 2016

That is what I'm trying to figure out and glean information from.

The documentation is a little confusing.

I'm remapping whatever is sent to it according to the order Safari needs. If that is wrong then it would be good to know although it seems to work. I don't believe there should be the need for two different configs if it can be done dynamically I think.

The method was just a static helper method to be added to your library to be used externally on the channelMap config. Not remapped internally. That's perfectly fine mate !

@hoch
Copy link
Member

hoch commented Aug 2, 2016

I'm remapping whatever is sent to it according to the order Safari needs. If that is wrong then it would be good to know although it seems to work. I don't believe there should be the need for two different configs if it can be done dynamically I think.

Yes, that is my point. If you want to add that static method into your wrapper-like utility, that is fine by me. Omnitone already supports the dynamic channel remapping and your wrapper is using it to remap the channels for Safari. I think this is quite natural to me.

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

No branches or pull requests

2 participants