search

Andersama / obs-asio

ASIO plugin for OBS-Studio
GNU General Public License v3.0
664 stars 43 forks source link

Resolves issue with OBS30 #144

Closed Lucy-Isabelle closed 6 months ago

Lucy-Isabelle commented 7 months ago

It seems in OBS30 they changed the way the functions work so fill_out_channels_modified never got triggered. I have modified the code slightly by manually calling this function on a device change as well as format change instead of using just obs_property_set_modified_callback

pkviet commented 7 months ago

As the original authors of this plugin, Andersama and I are in discussion regarding the future of this project. Meanwhile, I fixed the said bug in a new v4 of the plugin which is available here, which is lighter (80kB) and doesn't link to juce anymore: https://github.com/pkviet/obs-asio/releases/tag/4.0.0v3

As to the current PR, I'll just comment generally that:

There's a general CI issue with Inno Setup installer which results in the installer being flagged by Microsoft Defender. This will have to be concurrently fixed if CI builds are to be used for Release.

My v4 branch follows updated obs-plugintemplate otoh. The releases are based on NSIS as they used to be before the use of CI.

Lucy-Isabelle commented 7 months ago

Thank you for that information, and the general information is good. Apologies for not squashing the commits.

Honestly I didn't read your comment, but that's exactly what was happening. It seems OBS 30 changed the way that the set_modified_callback function was working as this worked in OBS28 so I just implemented a manual function call prior to the set_modified_callback addition on both change devices and change format callbacks.

The last thing I did was remove a redundant line of clearing the list as that was already in the filling channels function so it was happening twice.

Thank you for the link to the new plugin as well, I didn't know this was released and would have saved me a lot of time troubleshooting the plugin actually haha (So would have reading your comment)

As for not using JUCE in your version, can I ask what benefits that has? I apologise, but I've always just used this plugin.

Also happy to close this commit and not proceed with the PR as there is already fix that you've done as per your github and you're the original author

pkviet commented 7 months ago

As for not using JUCE in your version, can I ask what benefits that has? I apologise, but I've always just used this plugin.

  1. This means no more static linking to juce and way smaller binaries (80 kB).
  2. If Steinberg should change its licensing., this allows a transition to some day have obs-asio natively to obs-studio because juce would never ever be accepted as a dependency to obs-studio, I believe it's the right move forward so that's the only version I want to develop and maintain. But again, we are in discussions with Andersama. Maybe we can have a transition period between v3 and v4. We'll see.
Lucy-Isabelle commented 7 months ago

I believe it's the right move forward so that's the only version I want to develop and maintain. But again, we are in discussions with Andersama. Maybe we can have a transition period between v3 and v4. We'll see.

Entirely fair and I agree with this approach, taking out a third party is always the best option, thoughts about moving over once yours is out of Alpha? Only because, I know a lot of people who update to v30+ might be hesitant to move over to an alpha version as they may need stability and be worried incase it crashes mid recording/stream etc.

pkviet commented 7 months ago

Entirely fair and I agree with this approach, taking out a third party is always the best option, thoughts about moving over once yours is out of Alpha? Only because, I know a lot of people who update to v30+ might be hesitant to move over to an alpha version as they may need stability and be worried incase it crashes mid recording/stream etc.

it's alpha only in so far that it needs testers. So chicken and egg. It's stable enough for me. I'll wait a little while and if there's no reports I'll switch to official full release.

Andersama commented 7 months ago

@Lucy-Isabelle I'm not normally too actively checking on this plugin for various reasons, including as you kind of hinted at the plugin's been largely stable for years now. I haven't been an obs developer for a long time now and have not been following along with their releases, and pkv's been doing other things. So at this point it takes someone getting my attention that the abi has broken again, which isn't ideal. Your pull request is just awkwardly timed, since pkv's done a bit to rewrite the plugin yet again and things might be changing.

Ignoring all that I don't see any obvious problems with the commit.

Malumen commented 6 months ago

Anyone coming here seeing the link in this post no longer works, please go here instead:

https://github.com/pkviet/obs-asio/releases/

pkviet commented 6 months ago

Pushed the fix in 31f87ed9f63392d68906e12694f3e7a37d932376 Release with fix available in Release section.