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

db: Update database models for compatibility with 2.0.12 / HEAD #9

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

tuxuser
Copy link
Contributor

@tuxuser tuxuser commented Jan 29, 2022

This change makes hyperion.rs compatible with latest hyperion.ng database, so settings can be imported from hyperion.db and dumped into TOML-representation if desired.

#[validate(range(min = 10))]
pub width: u32,
#[validate(range(min = 10))]
pub height: u32,
pub fps: u32,
pub framerates: String,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's a list of framerates, maybe it should be a Vec<u32>? Do we have a spec/schema for this somewhere?

pub fps: u32,
pub framerates: String,
pub input: u32,
pub resolutions: String,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for framerates, this looks like a list, maybe it should be a Vec with Resolution suitably defined?

pub available_devices: String,
pub device: String,
#[serde(rename = "device_inputs")]
pub device_inputs: String,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below, this should be a list if it's a list of devices, not a String

src/models/global.rs Outdated Show resolved Hide resolved
src/models/global.rs Outdated Show resolved Hide resolved
src/models/global.rs Outdated Show resolved Hide resolved
src/models/instance.rs Outdated Show resolved Hide resolved
src/models/instance.rs Outdated Show resolved Hide resolved
@tuxuser
Copy link
Contributor Author

tuxuser commented Jan 30, 2022

When you are suggesting changes from String to Vec<..>, it would need a custom De-/serializer, correct?
Looking through the schemas in hyperion.ng they list the mentioned fields as "String".

That are:

  • framegrabber->available_devices
  • framegrabber->device_inputs
  • framegrabber->framerates
  • framegrabber->resolutions

Here's the schemas btw: https://github.com/hyperion-project/hyperion.ng/tree/master/libsrc/hyperion/schema

src/models/global.rs Outdated Show resolved Hide resolved
@alixinne
Copy link
Owner

When you are suggesting changes from String to Vec<..>, it would need a custom De-/serializer, correct? Looking through the schemas in hyperion.ng they list the mentioned fields as "String".

That are:

* framegrabber->available_devices

* framegrabber->device_inputs

* framegrabber->framerates

* framegrabber->resolutions

Here's the schemas btw: https://github.com/hyperion-project/hyperion.ng/tree/master/libsrc/hyperion/schema

Yes, I think we need (at least) a custom deserializer, maybe a serializer if we intend to write back to the database. I had a look at the schema and it does say string, but parts of the JS code for the UI seem to treat this as some kind of list? e.g.
https://github.com/hyperion-project/hyperion.ng/blob/2f573a117feb34d2c0de6143b0c49466622b45f8/assets/webconfig/js/content_grabber.js#L288

To give more context on the config loading, I went with the "make the parsing as strict as possible", so we only have to do the hard reverse engineering work once, and then the rest of the engine has ready-to-use explicit values (hence the enums, Options, etc. which are not present in the original schema).

@alixinne alixinne force-pushed the task/update/db_models branch 3 times, most recently from 2ec9949 to ef79296 Compare February 5, 2022 12:52
@tuxuser
Copy link
Contributor Author

tuxuser commented Feb 5, 2022

Did not have the time yet to look into those pseudo-list config options ..

@alixinne
Copy link
Owner

So here's what I gathered from a fresh 2.0.12 install:

{
 "available_devices": "UVC Camera (046d:0825)",
 "blueSignalThreshold": 0,
 "cecDetection": false,
 "cropBottom": 0,
 "cropLeft": 0,
 "cropRight": 0,
 "cropTop": 0,
 "device": "/dev/video1",
 "device_inputs": "0",
 "enable": true,
 "encoding": "YUYV",
 "flip": "NO_CHANGE",
 "fps": 15,
 "fpsSoftwareDecimation": 0,
 "framerates": "15",
 "greenSignalThreshold": 100,
 "hardware_brightness": 128,
 "hardware_contrast": 32,
 "hardware_hue": 0,
 "hardware_saturation": 32,
 "height": 480,
 "input": 0,
 "noSignalCounterThreshold": 200,
 "redSignalThreshold": 0,
 "resolutions": "8",
 "sDHOffsetMax": 0.46,
 "sDHOffsetMin": 0.4,
 "sDVOffsetMax": 0.9,
 "sDVOffsetMin": 0.1,
 "signalDetection": false,
 "sizeDecimation": 8,
 "standard": "NONE",
 "width": 640
}

It seems that even though the field names are plural, they only represent a single value in the UI (there's only one of each capture device type selected for any single instance), so they're not lists, and their type is as follows:

  • available_devices: String, friendly device name
  • device_inputs: usize (but stored as a String, needs parsing), index into the list of known V4L2 devices?
  • framerates: u32 (but stored as a String, needs parsing), number of frames per second, matches what's in the fps field
  • resolutions: usize (but stored as a String, needs parsing), index into the list of resolutions supported by the V4L2 device. The actual resolution is given by width and height

@tuxuser
Copy link
Contributor Author

tuxuser commented Feb 21, 2022

Thx for posting the json representation of the config section in question.

For the deserialization FromStr and serialization fmt::Display an additional dependency could be pulled in: https://crates.io/crates/serde_with

#[serde_as]
#[derive(Deserialize, Serialize)]
struct Foo {
    // Serialize with Display, deserialize with FromStr
    #[serde_as(as = "DisplayFromStr")]
    bar: u8,
}

@alixinne
Copy link
Owner

Thx for posting the json representation of the config section in question.

For the deserialization FromStr and serialization fmt::Display an additional dependency could be pulled in: https://crates.io/crates/serde_with

#[serde_as]
#[derive(Deserialize, Serialize)]
struct Foo {
    // Serialize with Display, deserialize with FromStr
    #[serde_as(as = "DisplayFromStr")]
    bar: u8,
}

Good idea, I wasn't aware of serde_with, it makes sense for these.

@tuxuser
Copy link
Contributor Author

tuxuser commented Feb 23, 2022

Framegrabber seems like the base struct and GrabberV4L2 derives from it.
The json you posted has fields from GrabberV4L2.

Now, to keep code duplication at a minimum, shall we pull in Framebuffer into GrabberV4L2 via serde-flatten ?

Incorrect, V4L2 does not inherit all the fields from Framebuffer... annoying!
So it needs fields re-defined...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants