bitfocus / companion-module-ptzoptics-visca

MIT License
8 stars 11 forks source link

Add preset and power feedbacks #9

Closed hjoelr closed 2 months ago

hjoelr commented 3 years ago

I've been finding a need to be able to know what preset the camera is currently at. Unfortunately the PTZ Optics visca over IP API does not offer a way to query the camera to know if it's currently on a preset. That would have made things significantly more simple. Instead, we have to keep track of the preset in the software. This PR is an attempt at making that possible.

In addition to the preset feedback, I've also added a feedback to display if the camera is on or off. Again, this cannot be queried from the camera, so the software cannot know the actual power state until a power on or off command is sent to the camera.

feedback-preset

I do not update the feedbacks until we receive an associated "command complete" response from the camera. This is to make the feedback more accurate to the actual state of the camera.

Because I have to hold state in the software, there are opportunities for the state to get out of sync if something weird happens. I've tried to account for everything I experienced in my testing against a real camera, so hopefully this won't happen. I think time will tell if there's a bug in here somewhere.

This PR also fixes an issue where the power on command would not work the first time it is used. In addition to that, this PR lays the foundation for enabling inquiry commands to update dynamic variables and also gives a power feedback as requested in #4.

hjoelr commented 3 years ago

I forgot to mention earlier that this adds dynamic variables for the power and currently active preset.

image

omeaart commented 3 years ago

I would made this feedback completely opt-in, and not include it in the default presets. Or at least create a separate set of presets that include/exclude the feedback.

We use this module, but also control the camera's with a joystick outside of companion. You can imagine the weird results you would get, if this is the default behaviour.

hjoelr commented 3 years ago

@omeaart Thanks for letting me know your workflow. I hadn't really considered handling changes outside of Companion. Maybe I need to re-think how I'm doing the feedback for these presets. In order to handle your scenario I would need to capture the position of a preset after it's recalled or saved in Companion and then continuously poll the camera to determine if it's camera position has changed.

@krocheck Three questions:

  1. Do you think frequently polling (say every second) the camera for it's position would be too resource intensive? This same concept would be needed for other (future) feedbacks. Each feedback type would represent a separate poll request to the camera. So, if this module supported 5 feedback types, it would require 5 requests to the camera each polling period (ie. every second).

  2. Maybe one way to reduce load is to only do the polling if the current bank has buttons with the feedbacks. Is there an event that's fired or some way the module could know whether one of the buttons on the current bank has one of it's feedbacks?

  3. Is there an API for modules to store data that can be recalled across program launches? I'm specifically thinking about storing the positions represented by the presets since these can only be learned when a preset is recalled or saved from Companion.

sjkav commented 2 years ago

Hi @hjoelr hjoelr - did this stuff ever make it into a build? I'd love to use the power & preset feedbacks, but I don't see this in the latest v2.2.0 ? I'm not a developer, so I'm struggling to work out how to get your extended functionality. I'd hoped it got built into the later versions...

hjoelr commented 2 years ago

@sjkav No, this has not made it into a build. The main reason is because of the concern by @omeaart that the feedbacks would become inaccurate if changes to the camera were made outside of Companion. I think that is very valid and probably would impact a significant number of users. I have not had time to work on reworking the code to better handle that scenario. I've also not had direction from @krocheck on direction to take with improving the feedback architecture for this.

@krocheck could you take a look at my question from November 30, 2020 and maybe help out with a recommendation on path forward with this?

jaylaw64 commented 2 years ago

Would it be possible to pull the commit 5c6fee8b6fee0e46b3d75ab0f6eb993ad17e053c out of this thread and merge it separately? The support for 128 presets is really unrelated to the feedback issue and I would love to see this supported.

Why do we need 128?

We have multiple camera operators for multiple events. Each one of them is assigned a range of presets they can use for their show within that range they can do whatever they want. so Operator #1 gets 1-20, Operator #2 gets 21-40, etc. so no one show uses 128 presets, but multiple shows do.

Thanks.

Gartom commented 2 years ago

Would it be possible to pull the commit 5c6fee8 out of this thread and merge it separately? The support for 128 presets is really unrelated to the feedback issue and I would love to see this supported.

Why do we need 128?

We have multiple camera operators for multiple events. Each one of them is assigned a range of presets they can use for their show within that range they can do whatever they want. so Operator #1 gets 1-20, Operator #2 gets 21-40, etc. so no one show uses 128 presets, but multiple shows do.

Thanks.

The presets are already available since #15, #16 and #17, but you need to run the beta builds of Companion to use them I believe.

jaylaw64 commented 2 years ago

Thanks for your prompt reply.I asked because I downloaded the new 2.1.4 release hoping this was present and it was not. I just downloaded a 2.2.0 Beta on a spare computer and the change is present.I've hesitated to put my stream (a church service for a few hundred folks) at risk of a Beta build just to get the extra presets. Is there any chance of a 2.1.X release with this change or a full 2.2.X release? There are lots of other things (variables in particular) that I would love to take advantage of.Sincerely,JayOn Feb 12, 2022, at 9:19 PM, Tomas Gardström @.> wrote:Would it be possible to pull the commit 5c6fee8 out of this thread and merge it separately? The support for 128 presets is really unrelated to the feedback issue and I would love to see this supported.Why do we need 128?We have multiple camera operators for multiple events. Each one of them is assigned a range of presets they can use for their show within that range they can do whatever they want. so Operator #1 gets 1-20, Operator #2 gets 21-40, etc. so no one show uses 128 presets, but multiple shows do.Thanks.The presets are already availablie since #15 and #17.—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.>

hjoelr commented 2 years ago

@jaylaw64 I understand your concern. I was afraid of using the beta version in production, but because they rarely update the release version, the beta ends up being the better build to use. We've been using beta in production for a while.

There is one particular quirk that I've noticed with the PTZOptics preset recall where it doesn't always fully recall for some presets and I haven't had time to dig into why. As a workaround, I found that one preset always works to recall (Preset 29 in my case), so I just added two recall actions in a row in Companion where the first action recalls the 29 preset and the second recalls the actual preset I need. This has been reliable. The camera does a little jig when doing this, so you don't want to do it when your camera is live. Aside from that, I've not had any noticeable issues with beta. My beta build is a couple months old.

jaylaw64 commented 2 years ago

Thank you for your reply.The kind of unpredictability you mention below is exactly what I'm trying to avoid. We have 30 camera presets for each of 3 cameras. Going through and setting all of them to go to a phantom position and then the real position is nightmare inducing. As the designer of the system, I can probably deal with it, but explaining it to our volunteers would be impossible.I am a professional software engineer and have been for over 35 years. I see new Beta builds multiple times per day. The chance of something bad being injected is massive. The relationship between code churn and bugs is well documented. As long as there are large changes, there will be new and fascinating bugs.I guess I'll just stick with what I have until the Powers That Be create a blessed 2.2.Thanks again,JayOn Feb 15, 2022, at 1:27 PM, Joel Rowley @.> @. I understand your concern. I was afraid of using the beta version in production, but because they rarely update the release version, the beta ends up being the better build to use. We've been using beta in production for a while. There is one particular quirk that I've noticed with the PTZOptics preset recall where it doesn't always fully recall for some presets and I haven't had time to dig into why. As a workaround, I found that one preset always works to recall (Preset 29 in my case), so I just added two recall actions in a row in Companion where the first action recalls the 29 preset and the second recalls the actual preset I need. This has been reliable. The camera does a little jig when doing this, so you don't want to do it when your camera is live. Aside from that, I've not had any noticeable issues with beta. My beta build is a couple months old.—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: @.***>

hjoelr commented 2 years ago

@jaylaw64 Yeah, I hear you. It sounds like your use case is a lot bigger than mine.

Gartom commented 2 years ago

I guess I'll just stick with what I have until the Powers That Be create a blessed 2.2.

@jaylaw64, I fully understand your concern of using betas in general. Anyway, I would like to balance up this dialogue a bit.

Our setup is similar as yours with a live church stream every Sunday since the pandemic hit us with restrictions in March 2020. We bought an ATEM TVS HD and ran the first half year of weekly streams with lots of volonteer camera operators and manually mixing the stream using ATEM SW Control. In November 2020, we bought two PTZOptics cameras and two weeks later we bought a Stream Deck to control them and the rest of the rig via Companion (the current release at that time) after fiddling with other solutions for controlling the cameras.

We quickly ran into the same type of issue as you have seen with the limited number of presets for the old PTZOptics module, so I made an effort of adding the needed presets in the PTZOptics module. We started out with a "home built" 2.2.0 with the added PTZOptics updates which worked very well for us and then I checked in the PTZOptics module updates and got them merged in to the main branch. We have not had any specific issues with the PTZOptics module, and I don't recognize the issue you mention with some presets not being recalled correctly, so that may be something that also has been fixed.

I also had concerns of running the "betas" for a live stream but after asking around in the Slack channels, the betas generally seem to be as good (and in many cases better) than the 2.1.4 release. I would say that the important thing is to try a build and test it extensively in a "live stream like" environment to get a feeling that it works OK.

jaylaw64 commented 2 years ago

Please let me preface this by saying I really appreciate all the hard work by the maintainers and contributors. I have 2 Streamdeck XL's, a 15-key Stream Deck, and I use the emulator on a separate computer during our services (I'm livestreaming for a church). I have a bunch of volunteers and the automation makes it possible for me to train almost anyone to run the video, audio, and graphics. I control vMix, a Behringer X32 w/ XDante, 3 PTZOptics cameras, and our overlay graphics for titles, introductions, lyrics, and readings. Thank You for a great piece of software!I have downloaded and tested it in a parallel environment using a separate streaming computer with all the real cameras, audio, etc. It works quite well. I would be especially excited to start using variables because I have lots of places where I use multiple buttons where a single button with state behind it would do the job better.However, my experience has shown that bugs do not show up by just running through a singe mock service. They might show up: by running through it 3 times without a reboot; or running at a specific time of day; or be state-specific where someone has to to A followed by B, or maybe just a really slow memory leak that you wouldn't notice for days until Boom ...As long as incremental enhancements are made in subsequent betas there is high risk that any of these could be introduced. A stable release that excepts only bug fixes is the only way to get something that approaches stable software (no software is truly stable).Is there a plan for a 2.2 stream release? It appears to have been worked on for a long time.Thanks,JayOn Feb 16, 2022, at 1:47 PM, Tomas Gardström @.> wrote:I guess I'll just stick with what I have until the Powers That Be create a blessed 2.2. @jaylaw64, I fully understand your concern of using betas in general. Anyway, I would like to balance up this dialogue a bit.Our setup is similar as yours with a live church stream every Sunday since the pandemic hit us with restrictions in March 2020. We bought an ATEM TVS HD and ran the first half year of weekly streams with lots of volonteer camera operators and manually mixing the stream using ATEM SW Control. In November 2020, we bought two PTZOptics cameras and two weeks later we bought a Stream Deck to control them and the rest of the rig via Companion (the current release at that time) after fiddling with other solutions for controlling the cameras.We quickly ran into the same type of issue as you have seen with the limited number of presets for the old PTZOptics module, so I made an effort of adding the needed presets in the PTZOptics module. We started out with a "home built" 2.2.0 with the added PTZOptics updates which worked very well for us and then I checked in the PTZOptics module updates and got them merged in to the main branch. We have not had any specific issues with the PTZOptics module, and I don't recognize the issue you mention with some presets not being recalled correctly, so that may be something that also has been fixed.I also had concerns of running the "betas" for a live stream but after asking around in the Slack channels, the betas generally seems to be as good (and in many cases better) than the 2.1.4 release. I would say that the important thing is to try a build and test it extensively in a "live stream like" environment to get a feeling that it works OK.—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: @.>

Gartom commented 2 years ago

@jaylaw64, I understand your point of view, I just wanted to throw in my two cents into the discussion. I have 25+ years of engineering experience in mobile device consumer and automotive telematics business, both in designing systems, developing software and hardware and I fully agree that a real release where you draw a line for functionality and do extensive testing (and fix all found bugs in pre-release testing) is the way to go.

For me, the live church stream does not have the same type of requirements as e.g. an automotive ECU with embedded SW with expected lifetime of 15+ years and extensive environmental requirements. Therefore, I don't put the same requirements on our church live stream system and we have made a judgement that the setup we have is well over "good enough" . As any system is not better than the weakest link, we don't know what the developers of every piece of equipment used as guideline for their releases. For Companion, it is possible to see everything that has changed, and as much of the functionality is in modules we don't use, they have no impact in runtime.

We start up our system from scratch each Sunday morning following a start-up checklist, and by doing so, we get a clean start with known status. What I wanted to highlight with the post above was that we have run our Sunday streams on 2.2.0 betas since March last year without any big issues. We have seen (and fixed) some issues, but we saw more issues with the 2.1.0 and 2.1.1 release we started to use before swapping to betas.

If you want to minimize the risk, you could actually build your own Companion system using the 2.1.4 release based on the source code on Github. If you then add the PTZOptics visca module from Github in the "module-local-dev" folder and build the system, you will get the benefit of all the presets (and bug fixes) in the PTZOptics module while keeping the old Companion 2.1.4 base release.

Gartom commented 2 years ago

Just realized that there is a release planned for 2.2 😀

https://bitfocusio.slack.com/archives/CFG7HAN5N/p1643063527193900?thread_ts=1643063527.193900&cid=CFG7HAN5N

jaylaw64 commented 2 years ago

Awesome! My dreams are coming true!On Feb 16, 2022, at 4:08 PM, Tomas Gardström @.> wrote:Just realized that there is a release planned for 2.2 😀https://bitfocusio.slack.com/archives/CFG7HAN5N/p1643063527193900?thread_ts=1643063527.193900&cid=CFG7HAN5N—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: @.>

jswalden commented 2 months ago

For better or worse, this PR is old, and many parts of it are outdated. Some parts already landed, other parts maybe got fixed independently, many bits no longer apply. Especially after a v3 conversion, a TypeScript conversion, and the adding of full VISCA command/inquiry-response handling completely different from what was proposed here. (Although some of these changes aren't released yet.)

Besides which, the admitted-here ignoring of other concurrent mutators seems...well, moderately troubling. It's gotta be pretty common to want to control a camera via Companion, but also to have and at least occasionally use a control pad for fine-tuning. These cameras and their API are just not designed to maintain some sense of their current state as in a current active preset, given that they don't send regular feedback as actions are taken. And presets are more like "set these settings" than persistent states you put the camera in.

Anyway. All of this sums up to reason for me as maintainer to close this (with the regret any reviewer has rejecting long-neglected changes someone obviously went to a bunch of effort to implement).

I think it would not be too offensive to the VISCA paradigm to maintain a "last preset called" variable in the module (that would, as the label on the tin says, never reflect changes made out-of-band), if someone wants to submit that fresh PR. But I don't think I see anything else from this PR that I'd accept a revised patch for.

hjoelr commented 1 month ago

@jswalden Thanks for your considerate comment and feedback on the PR. I think closing it is the right move for the reasons stated. While I did put a good amount of effort into this back at the time, part of the work was for education, which is not lost. I've not dug into the Companion code since the changes to TypeScript, but hope to someday and maybe to implement the suggested functionality.