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

megapixels: init at 0.14.0 #98479

Merged
merged 1 commit into from Jan 18, 2021
Merged

Conversation

OPNA2608
Copy link
Contributor

@OPNA2608 OPNA2608 commented Sep 22, 2020

Motivation for this change

Packaging a well-behaving camera app that can use both of the PinePhone's cameras without any kernel hacks.

(Big) photo of the working app

image.jpg

Unsure where to put the package. It queries /proc/device-tree/compatible to figure out the device & available camera hardware, but configuration files in various locations may be used to override the detection - is that too os-specific? I don't know what else is can run on. Would pkgs/applications/video be better? Support for other devices may follow in the future, the shipped configuration files only describe the various PinePhone models right now.

Reviewers with a PinePhone running NixOS would be highly appreciated if possible! I tested this on the pre-installed postmarketOS distro of my Community Edition 3GB model. Compiles on my desktop, but no configuration to describe a usable camera here so it won't exactly do much.

I'm considering writing a NixOS module to configure /etc/megapixels.ini in case the user would like to override the camera settings on a system-level, would that make sense?

cc @samueldr for mobile-nixos, @TilCreator for recently having worked on better PinePhone support

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions (postmarketOS)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@OPNA2608
Copy link
Contributor Author

OPNA2608 commented Sep 29, 2020

Bumped to 0.9.0. More buttons have been added, one of them for opening the last taken image and the photo directory in their default applications. This silently fails on my phone. Will look into it more when I got NixOS on it. The code in question looks e.g. like this:

sprintf(uri, "file://%s", last_path); /* path to a JPEG file */
if(!g_app_info_launch_default_for_uri(uri, NULL, &error)){
	g_printerr("Could not launch image viewer: %s\n", error->message);
}

@OPNA2608 OPNA2608 changed the title megapixels: init at 0.9.0 megapixels: init at 0.10.2 Oct 2, 2020
@OPNA2608
Copy link
Contributor Author

OPNA2608 commented Oct 2, 2020

Bumped to 0.10.2. Added optional dependencies on libraw and imagemagick for converting the taken photos to tiff and jpg.

@Pacman99
Copy link
Contributor

Pacman99 commented Oct 19, 2020

LGTM I'll try and test it on my pinephone later, but it ran successfully. Thanks for packaging this!

Result of nixpkgs-review pr 98479 1

1 package built:
  • megapixels

@OPNA2608
Copy link
Contributor Author

0.11.1 has been released in the meantime, and releases have slowed down abit. It adds very nice-to-have post-processing. I can do the bump itself without the extra deps, but I'd like to package them first. Waiting for halide: 2019.08.27 -> 10.0.0, enable on aarch64-linux so I can open a PR for HDR+.

@OPNA2608 OPNA2608 mentioned this pull request Nov 5, 2020
10 tasks
@SuperSandro2000
Copy link
Member

If you address the one review commit I am going to merge it and we can do the bump later. Feel free to ping me or write me on IRC about it.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 98479 run on x86_64-linux 1

1 package built:
  • megapixels

@OPNA2608
Copy link
Contributor Author

OPNA2608 commented Nov 27, 2020

@SuperSandro2000 I've bump it to the latest release without the hdr-plus dependency & tested build on x86_64-linux (doesn't run properly, not its intended environment). Building on my phone takes too long for me right now, but the only difference to what I'm using there is the optional hdr-plus dependency so I assume it'd run, just with worse image quality.

@OPNA2608 OPNA2608 changed the title megapixels: init at 0.10.2 megapixels: init at 0.12.0 Dec 6, 2020
@fx-chun
Copy link
Member

fx-chun commented Dec 26, 2020

Would be helpful to get this packaged!

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 98479 run on x86_64-linux 1

1 package built:
  • megapixels

@OPNA2608
Copy link
Contributor Author

I'll bump it to 0.13.2, just building it for my phone to test it really quickly.

@OPNA2608 OPNA2608 changed the title megapixels: init at 0.12.0 megapixels: init at 0.13.2 Dec 27, 2020
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 98479 run on x86_64-linux 1

1 package built:
  • megapixels

@OPNA2608 OPNA2608 changed the title megapixels: init at 0.13.2 megapixels: init at 0.14.0 Jan 14, 2021
@Pacman99
Copy link
Contributor

Result of nixpkgs-review pr 98479 run on x86_64-linux 1

1 package built:
  • megapixels

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 98479 run on x86_64-linux 1

1 package built:
  • megapixels

@SuperSandro2000 SuperSandro2000 merged commit f97266e into NixOS:master Jan 18, 2021
@OPNA2608 OPNA2608 deleted the package-megapixels branch September 27, 2022 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants