termux / termux-api-package

Termux package containing scripts to call functionality in Termux:API.
MIT License
1.01k stars 317 forks source link

Improving the camera app #188

Open raduprv opened 1 month ago

raduprv commented 1 month ago

I really like the camera command line program, I've been looking for something like this for a long while. However, the options are very limited, and it also has a small bug, where the max resolution stops at 12 MP rather than 27 MP which is what my phone camera outputs.

I fixed that bug, and started to implement other features, like being able to change the focus distance, iso, exposure, etc. Is there any interest for this code, and would you like it merged with the termux-camera app? If so I can submit a patch soon-ish. Unfortunately, I am not good at sh scripting, so I can't modify the script that calls the API to add extra options (I am directly invoking the API from command line, and it's a bit ugly). I could use a bit of help on how to pass extra optional arguments.

Grimler91 commented 1 month ago

Hi, any improvements are very welcome, and the camera script could indeed need some improvements.

If you open a PR with suggestions for changes then we can help test, review and propose additional improvements on top of it, thanks!

agnostic-apollo commented 1 month ago

Probably passing additional config options would be doable, but camera and other APIs needs a complete rewrite and need to use a foreground service to actually work in newer android versions (especially if started from background), which are planned for future. Video API to be added would also require a service to maintain state.

https://developer.android.com/about/versions/11/privacy/foreground-services

raduprv commented 1 month ago

But in the link you provided it says you can't use the camera if started by a foreground service started in the background. What I like about the current implementation is how you can take photos with the screen off, via ssh (or via some script). This way, old phones can be repurposed for photography things, such as doing timelapse videos, focus bracketing, etc.

agnostic-apollo commented 1 month ago

If you normally start the app that starts the foreground service, then you can use that service to record while app is in background. There is no way around that on android >= 11, so new design is necessary to be implemented. Older versions aren't affected and will work fine with new design too, but video recording isn't possible without a service, broadcast receivers currently used only has a few seconds before android kills it, so it cannot be used for any long term API, even photos could potentially fail if camera setup takes extra time.

https://github.com/termux/termux-api/issues/514#issuecomment-1128727763

raduprv commented 1 month ago

Yes, I am aware of that restriction where android kills it if the job is not done fast. I encountered that problem with a timelapse app I wrote, where the alarm receiver would be killed by Android before finishing the job (it needed some seconds because it did other things as well). That problem can be fixed with root though (there is a command to increase that limit for all processes).

agnostic-apollo commented 1 month ago

Lot of things can be done with root, but since app is primarily meant for non-rooted users, the design needs to be fixed. The whole app and package scripts need a rewrite actually for many many issues, will be done eventually in future, there are some blockers actually before work can be started, some libraries need to be implemented for the API management so that they can be used for both termux-app and termux-api.

raduprv commented 1 month ago

I haven't checked everything, and I am not an Android developer (just doing this as a hobby) but I thought the reason termux moved to f-droid was because some things can't be done the google way. From my observations, Google (and in some cases the phone makers too) like to make things more and more restrictive. My Oppo phone for example sometimes likes to kill whatever Termux runs, complaining about security issues and the like. Even with the background limit disabled.

agnostic-apollo commented 1 month ago

Background process limit in developer options is unrelated and so is broadcast receiver runtime. Many vendors have aggressive killers for app and child processes, some can be disabled, especially with root. You may wanna read:

Google PlayStore doesn't allow apps to have lot of dangerous permissions and their review process is random, F-Droid doesn't have that issue, especially with Termux being open source.

raduprv commented 1 month ago

So after the new magic is done to make Termux compatible with newer Android versions, will it still be possible to start Termux, start sshd, then take pictures with the phone with the screen off?

agnostic-apollo commented 1 month ago

I think should be possible, will see.

raduprv commented 1 month ago

Well anyway, so far I added support for ISO, Exposure, Focus. Today I will also add support for disabling image processing (NR and sharpening). I will also add support for WB and for choosing the preview time (500 ms seems a bit excessive in most cases). I also fixed the max resolution problem (using a more reliable way to get the max resolution).

Now, I am not a Java programmer (I am a low level guy, C, Assembly and the like) so I have a question: I added a bunch of public static variables in the CameraPhotoAPI class. The alternative is to pass them in function parameters, but there are two functions and it will get ugly with so many variables. Is it OK to keep them public static, or do you want me to do it some other way?

agnostic-apollo commented 1 month ago

If variables are not going to be used by other classes, then use private static.

raduprv commented 1 month ago

Ok, so I implemented quite a few things, but I plan to do more. Should I clean up the code and submit a patch? Or should I implemented everything I want to, and then make a patch? Some things will take a while to do.

agnostic-apollo commented 1 month ago

Whatever works for you. Note that currently I don't have time to review as other work is going on, so merging will take time depending on how big your changes are and when I get time.

raduprv commented 1 month ago

Ok, I created PR, I hope I did it right: https://github.com/termux/termux-api/pull/694 Please let me know if you want me to make any changes.