jniemann66 / ReSampler

High quality command-line audio sample rate converter
GNU Lesser General Public License v2.1
160 stars 26 forks source link

Re: more elegant cleanup of androidbuf upon exit #19

Closed jniemann66 closed 5 years ago

jniemann66 commented 5 years ago

Just doing a bit of cleanup. Do we need to have those raw deletes of the androidbuf buffers before every exit point in main() ?

Can we just do an RAII version using a smart pointer ? eg: std::cout.rdbuf(std::unique_ptr(new androidbuf(ANDROID_LOG_INFO, "ReSampler"))); // and remove the deletes etc

Originally posted by @mgood7123 in https://github.com/jniemann66/ReSampler/issues/8#issuecomment-511168566

jniemann66 commented 5 years ago

actually, I can see why it is done the way it currently is. just deleting the androidbuf is not enough. upon exit, the rdbuf of cout and cerr needs to be restored to whatever it was before.

Probably need to write a simple RAII manager class to clean-up correctly upon destruction. ... or maybe write a custom deleter that restores the original buffer etc.

I give up - Maybe it's just best they way it is :-)

jniemann66 commented 5 years ago

another idea is to simply use atexit() to clean-up. just put the delete std::cout.rdbuf(0); delete std::cerr.rdbuf(0); in some global function and apply it to atexit()

It would be called every time exit() is called. I think that would work.

jniemann66 commented 5 years ago

@mgood7123 check out the development branch https://github.com/jniemann66/ReSampler/commit/6d80de55fbc924610dc00460c82a1f1030f03203

mgood7123 commented 5 years ago

another idea is to simply use atexit() to clean-up. just put the delete std::cout.rdbuf(0); delete std::cerr.rdbuf(0); in some global function and apply it to atexit()

It would be called every time exit() is called. I think that would work.

But exit() cannot be called in android as it would terminate the apk itself aswell as the ReSampler however return is still viable according to https://www.systutorials.com/docs/linux/man/3-atexit/

The atexit() function registers the given function to be called at normal
process termination, either via exit(3) or via return from the program's main().
mgood7123 commented 5 years ago

Also sorry about being away, i have been trying to develop a VST plugin system for my application

jniemann66 commented 5 years ago

ok thanks - I'll give it some more consideration

jniemann66 commented 5 years ago

The atexit() function registers the given function to be called at normal process termination, either via exit(3) or via return from the program's main().

ok, so just reviewing the existing ReSampler code, exit() is never used, but return from main() is, so assuming atexit() on Android is invoked by returning from main(), we should be ok to use atexit() after all. Am I understanding this correctly ?

Also, I think I will soon "library-ify" ReSampler by decoupling the stuff in main() from the actual convert() function, with the latter residing in ReSampler.cpp and the former in a new file main.cpp Following that, I can then build everything except main() as a static (or dynamic ?) library and you (and I for that matter) can use it as a library and link directly to your app. (You would just stuff a ConversionInfo object with the desired parameters and call convert() etc )

Its just going to take a lot of messing around with makefiles etc and I don't have the stomach for it just yet :-)

jniemann66 commented 5 years ago

@mgood7123 I'm still not sure what to do about redirecting std::cout to androidbuf. If you are using ReSampler as a library, and explicitly calling main() then in this context, main() would just be an ordinary function, and thus atexit() won't be invoked. But then, if you are using ReSampler as a library, shouldn't it be the host app's responsibility to redirect std::cout anyway ?

Looks like you got the VST working - that's pretty cool. In the back of my mind, I have also been thinking about getting a VST host going myself for a while. Cheers Judd

mgood7123 commented 5 years ago

Yes but most android applications seem to just use logcat to print their messages, also I guess at exit would not be called if used as a library Sent from my Samsung Galaxy smartphone. -------- Original message --------From: Judd Niemann notifications@github.com Date: 21/7/19 11:29 am (GMT+10:00) To: jniemann66/ReSampler ReSampler@noreply.github.com Cc: mgood7123 smallville7123@gmail.com, Mention mention@noreply.github.com Subject: Re: [jniemann66/ReSampler] Re: more elegant cleanup of androidbuf upon exit  (#19) @mgood7123 I'm still not sure what to do about redirecting std::cout to androidbuf. If you are using ReSampler as a library, and explicitly calling main() then in this context, main() would just be an ordinary function, and thus atexit() won't be invoked. But then, if you are using ReSampler as a library, shouldn't it be the host app's responsibility to redirect std::cout anyway ? Looks like you got the VST working - that's pretty call. In the back of my mind, I have been thinking about getting a VST host going myself for a while. Cheers Judd

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread. [ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/jniemann66/ReSampler/issues/19?email_source=notifications\u0026email_token=AGLITH5QGMIWQVQVPMJEHH3QAO3WDA5CNFSM4IDWGU5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NY74A#issuecomment-513511408", "url": "https://github.com/jniemann66/ReSampler/issues/19?email_source=notifications\u0026email_token=AGLITH5QGMIWQVQVPMJEHH3QAO3WDA5CNFSM4IDWGU5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NY74A#issuecomment-513511408", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

jniemann66 commented 5 years ago

@mgood7123 have a look at the latest dev branch. I changed it around a bit.

The androidbuf stuff is still in main() but you are also free to call ReSampler::runCommand() if you wish ... I want to merge that into master but just checking in with you that it's not gonna break everything.

mgood7123 commented 5 years ago

@mgood7123 have a look at the latest dev branch. I changed it around a bit. The androidbuf stuff is still in main() but you are also free to call ReSampler::runCommand() if you wish ... I want to merge that into master but just checking in with you that it's not gonna break everything.

ill try to have a look when I can, im currently busy trying to get a Compositor (and then a Window Manager) working for my VST's

https://github.com/mgood7123/AndroidCompositor/blob/master/app/src/main/jni/jniapi.cpp

as once that is working I can then start moving the ReSampler and my WaveForm View into VST's and then try to create an Audio Processsing Pipeline (if that is the correct term) which would look something like this

Media Player > Pre Processing VST's > Resampler (required) > Post Processing VST's > Audio Out

mgood7123 commented 5 years ago

tho i guess i could start working on this now since the ReSampler is classified as an Effects plugin since it offers no GUI

mgood7123 commented 5 years ago

ill try it today

jniemann66 commented 5 years ago

no worries - I was just checking-in so that I didn't mess things up for you, since I made some reasonably significant changes. I'm gonna try and crank up my Android Studio later today.

also, trying to improve the cmake configuration - working on improving my cmake chops etc,

mgood7123 commented 5 years ago

Ok

mgood7123 commented 5 years ago

Well so far this works without issues

jniemann66 commented 5 years ago

I'm gonna close now :-)