-
-
Notifications
You must be signed in to change notification settings - Fork 7
Add picture quality settings and variables #71
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,12 @@ | ||
| const PICTURE_SETTINGS = [ | ||
| { id: 'color', label: 'Color', default: 50, min: 0, max: 100 }, | ||
| { id: 'brightness', label: 'Brightness', default: 40, min: 0, max: 50 }, | ||
| { id: 'contrast', label: 'Contrast', default: 90, min: 0, max: 100 }, | ||
| { id: 'sharpness', label: 'Sharpness', default: 50, min: 0, max: 100 }, | ||
| ] | ||
|
|
||
| module.exports = { | ||
| PICTURE_SETTINGS, | ||
| initActions: function () { | ||
| let self = this | ||
| let actions = {} | ||
|
|
@@ -151,6 +159,35 @@ module.exports = { | |
| }, | ||
| } | ||
|
|
||
| PICTURE_SETTINGS.forEach((setting) => { | ||
| actions['set_' + setting.id] = { | ||
| name: setting.label, | ||
| options: [ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may as well add the step setting they provide and populate the option with that too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, done.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was sort of meaning pull it out of their config via the API, but maybe that's more effort than it's worth. |
||
| { | ||
| type: 'number', | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know the right answer here, but whilst number is technically correct, it sadly currently excludes variable support, meaning people couldn't e.g. use this to set the value via a rotary encoder.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think we should do?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly I don't know, I think our three options are:
Maybe 1 makes sense just to start going for now. I think Companion centrally may improve the behaviour around that soon (it's just a shame they haven't done so already...). |
||
| label: setting.label, | ||
| id: 'value', | ||
| default: setting.default, | ||
| min: setting.min, | ||
| max: setting.max, | ||
| step: 1, | ||
| }, | ||
| ], | ||
| callback: async function (action) { | ||
| let opt = action.options | ||
| let params = { | ||
| settings: [ | ||
| { | ||
| target: setting.id, | ||
| value: opt.value.toString(), | ||
| }, | ||
| ], | ||
| } | ||
| self.sendCommand('video', 'setPictureQualitySettings', params) | ||
| }, | ||
| } | ||
| }) | ||
|
|
||
| self.setActionDefinitions(actions) | ||
| }, | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||||||||
| const { combineRgb } = require('@companion-module/base') | ||||||||||||||
| const { PICTURE_SETTINGS } = require('./actions') | ||||||||||||||
|
|
||||||||||||||
| module.exports = { | ||||||||||||||
| initPresets: function () { | ||||||||||||||
|
|
@@ -313,6 +314,38 @@ module.exports = { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| const PICTURE_SETTINGS = [ | ||||||||||||||
| { id: 'color', label: 'Color', default: 50 }, | ||||||||||||||
| { id: 'brightness', label: 'Brightness', default: 40 }, | ||||||||||||||
| { id: 'contrast', label: 'Contrast', default: 90 }, | ||||||||||||||
| { id: 'sharpness', label: 'Sharpness', default: 50 }, | ||||||||||||||
| ] | ||||||||||||||
|
|
||||||||||||||
|
Comment on lines
+317
to
+323
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given we're now using the ones from actions:
Suggested change
|
||||||||||||||
| PICTURE_SETTINGS.forEach((setting) => { | ||||||||||||||
| presets[setting.id] = { | ||||||||||||||
| category: 'Picture Settings', | ||||||||||||||
| name: setting.label, | ||||||||||||||
| type: 'button', | ||||||||||||||
| style: { | ||||||||||||||
| text: setting.label, | ||||||||||||||
| size: '14', | ||||||||||||||
| color: foregroundColor, | ||||||||||||||
| bgcolor: foregroundColorBlack, | ||||||||||||||
| }, | ||||||||||||||
| steps: [ | ||||||||||||||
| { | ||||||||||||||
| down: [ | ||||||||||||||
| { | ||||||||||||||
| actionId: 'set_' + setting.id, | ||||||||||||||
| options: { value: setting.default }, | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I normally provide say 5 presets from 0-100 with those values in the option and on the button (plus the feedback to show they're selected). Personally for me this preset is a bit pointless on it's own (and hard to test as an end user, as nothing will change). |
||||||||||||||
| }, | ||||||||||||||
| ], | ||||||||||||||
| up: [], | ||||||||||||||
| }, | ||||||||||||||
| ], | ||||||||||||||
| } | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| self.setPresetDefinitions(presets) | ||||||||||||||
| }, | ||||||||||||||
| } | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make volumeLevel NaN too if this is the best bet? Or undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since 0 has meaning, I set it to NaN as the default value, but perhaps it should be undefined instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefinedfeels a better fit to me. Can you update volume too to match.