Skip to content

Add JSON-API support for requesting image snapshots #1839

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

Merged
merged 10 commits into from
Mar 20, 2025

Conversation

xIronic
Copy link
Contributor

@xIronic xIronic commented Feb 20, 2025

Summary

This PR adds support for requesting a snapshot of the current screen via the JSON-API. By sending the "getImageSnapshot" command, the API responds with a base64-encoded JPEG image.
Alternatively, one can a request a snapshot of the current LED data for a given instance.

For example, this could be a message sent to the JSON-API:

Request: getImageSnapshot

{
   "command":"instance-data",
   "subcommand":"getImageSnapshot"
}

Response:

{
    "command": "instance-data-getImageSnapshot",
    "info": {
        "data":"<removed base64 string>",
        "format": "PNG",
        "height": 150,
        "width": 240
	},
	"success": true,
	"tan": 0
}

Request: getLedSnapshot

{
   "command":"instance-data",
   "subcommand":"getLedSnapshot"
}

Response:

{
    "command": "instance-data-getLedSnapshot",
    "info":{"leds":[[75,51,12],[84,66,28],[90,67,21],[93,67,21],[97,62,12]]},
    "success": true,
    "tan": 0
}

The main purpose of this feature is to support an external script that analyses time-shifted snapshots to detect whether my USB grabber has crashed. If a crash is detected, the grabber is reset, and Hyperion is restarted.

This approach eliminates the need for V4L2 loopbacks, as only one process can access my /dev/video* device at a time.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of web configuration, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing setups:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's body (e.g. Fixes: #xxx[,#xxx], where "xxx" is the issue number)

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated (docs/docs/en)
  • Related tests have been updated

PLEASE DON'T FORGET TO ADD YOUR CHANGES TO CHANGELOG.MD

  • Yes, CHANGELOG.md is also updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

If this feature is of interest to others and is likely to be merged, I will update the documentation accordingly.

@Lord-Grey Lord-Grey self-requested a review February 21, 2025 19:56
@Lord-Grey
Copy link
Collaborator

@xIronic Thank you for your PR and contribution!
Very much appreciated.

After having an initial review, let me share some feedback/considerations.
Let‘s explore jointly how we can round up the PR beyond the scenario you started with.

a) having the ApI responding with image data is perfectly fine, as it fits the request for that data.
Unfortunately, you write the same data to the server‘s filesystem in parallel which is not in line how the API is designed.
I would suggest removing the saving part from the PR.
It should be easily possible to handle requests, responses and comparison via a Python script and trigger corresponding actions. Just check with CoPilot or ChatGTP for a proposal.
Such an solution approach would be fully in line how API requests are to be used.

b) It would be great, if you could add an optional „instance“ element (see other Schemas for reference). The users with multiple instances can choose where to pick a snapshot from

c) You might want to allow passing the output „format“ of the returned image.
Just add an enumeration of format strings Qt provide. https://doc.qt.io/qt-6/qimagereader.html#supportedImageFormats

d) I am still thinking, if it might make sense to change the command to „instance-data“
and have subcommands like „getImageSnapshot“ and „getLedSnapshot“ and maybe others.
That would allow grouping multiple commands of the same nature logically. An image snapshot might be only one scenario currently .
What do you think?

Besides storing the image on disk, all other comments are to support scenarios that other user might benefit from on top of your provided code.

@Lord-Grey Lord-Grey changed the title Add JSON-API support for requesting and saving image snapshots Add JSON-API support for requesting image snapshots Feb 21, 2025
@xIronic
Copy link
Contributor Author

xIronic commented Feb 22, 2025

@Lord-Grey
Thank you for your feedback on my first PR, it was a really helpful and detailed insight.
I will try to address each of the points you made in your response.

a) Not doing any additional I/O on the server filesystem seems like a good approach to me. I will remove the path attribute. In fact, my script already uses requests and responses to avoid sd card wear.

b) Right, I forgot that the user can choose the capture source based on the instance. I will change it accordingly.

c) I've had this idea before and I like it. For example, the user could choose a format that is not compressed further.

d) This makes sense and the command would be more in line with the API. When I have some spare time, I will try to rewrite my API extension to be more comprehensive and extensible.

@xIronic
Copy link
Contributor Author

xIronic commented Feb 23, 2025

@Lord-Grey
I have rewritten the code to take your suggestions into account! Let me know what you think and if there are too many unnecessary commits.

The last commit fixed some formatting issues in JsonApi.cpp that my vscode introduced. I am still working out how to adapt it to an existing formatting standard a given file has.

@xIronic xIronic requested a review from Lord-Grey February 23, 2025 21:09
@Lord-Grey
Copy link
Collaborator

@Lord-Grey I have rewritten the code to take your suggestions into account! Let me know what you think and if there are too many unnecessary commits.

The last commit fixed some formatting issues in JsonApi.cpp that my vscode introduced. I am still working out how to adapt it to an existing formatting standard a given file has.

Thank you for your time incorporating the proposed changes.
There are some minor updates required to ensure consistency, plus I committed an updated JSON-API file as the LED-Data snapshot will not work otherwise. The background is given in the review comments above.

@Lord-Grey
Copy link
Collaborator

@xIronic I just did the two outstanding code changes that we can put the PR to bed.
Is there anything you would like to add or can the PR be merged?

@Lord-Grey Lord-Grey merged commit 37c141a into hyperion-project:master Mar 20, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Core Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants