blakeblackshear / frigate

NVR with realtime local object detection for IP cameras
https://frigate.video
MIT License
18.78k stars 1.7k forks source link

Disable zooming for PTZ #8496

Closed felipecrs closed 11 months ago

felipecrs commented 11 months ago

Describe what you are trying to accomplish and why in non technical terms My camera does not support zoom with PTZ. I would like to put something like:

cameras:
  my_cam:
    onvif:
      ptz_capabilities:
        zoom: false

So that neither the Frigate UI would show the zoom buttons nor the Frigate Card (unreleased but working nice).

Describe the solution you'd like Described above.

Describe alternatives you've considered Maybe there is some way to hide the zoom buttons from the Frigate Card.

Additional context N/A

NickM-27 commented 11 months ago

If your camera reports that it supports zoom but doesn't that is a camera firmware issue. I don't think we want to have manual configs to override what the camera reports it supports, then users may expect a zoom button to show when they set this to true even though the camera says it doesn't support it via onvif

NickM-27 commented 11 months ago

cc @hawkeye217

felipecrs commented 11 months ago

That should be the case. And it is unfortunately. I would never expect a no-brand Chinese camera to release a fix even if I report it to them.

Maybe we can name the config better to avoid confusion?

cameras:
  my_cam:
    onvif:
      ignored_capabilities:
        zoom: true
hawkeye217 commented 11 months ago

I agree with @NickM-27, Frigate should display controls for whatever the camera's firmware says it supports. Config entries for smaller UI elements like this may have the potential to add some confusion (especially for novice users).

hawkeye217 commented 11 months ago

As an alternative, you could use a browser extension to hide the element from the page.

felipecrs commented 11 months ago

Alright. Just to clarify, would you accept a PR for this feature (i.e. you just don't want to spend time on it), or would not even a PR be welcome?

As an alternative, you could use a browser extension to hide the element from the page.

That's not really an option for me. But thanks anyway.

NickM-27 commented 11 months ago

The initial effort of implementing it is not the issue. The issue is maintaining the option in future versions of frigate and the fact that there is a policy that we do not want to have lots of small config options that tweak the webUI, especially when those options only exist to get around camera bugs.

This might work as a browser feature where the webUI itself offers the ability to customize and keeps track of what ptz options to show and stores that in local browser storage

felipecrs commented 11 months ago

Got it. Well, an option to hide from the Frigate UI would not make much difference for me. What would is the ability of cascading such option to all frontends, specially the Frigate Card.

I'm closing this issue for now then, thanks for the discussion.

felipecrs commented 11 months ago

The Frigate Card already has the ability to hide zoom buttons btw:

live:
  controls:
    ptz:
      hide_zoom: true
felipecrs commented 3 months ago

Given the new UI and the new "focus" on UX, is there a chance this can be reconsidered?

hawkeye217 commented 3 months ago

Not sure anything has changed since @NickM-27's comment above:

The initial effort of implementing it is not the issue. The issue is maintaining the option in future versions of frigate and the fact that there is a policy that we do not want to have lots of small config options that tweak the webUI, especially when those options only exist to get around camera bugs.

felipecrs commented 3 months ago

Sorry about it. Reason I brought it back is because my workaround was to use the Frigate card, which has such option. However, with the new Frigate UI, maybe I will use it more.

Anyway, I think we can all converge that people shouldn't trash their cameras just because zoom capability cannot be disabled in Frigate if they want to get an "optimal" UX.

ONVIF should be one of the most badly interpreted protocols/specifications out there. The majority of camera firmwares have misinterpretations, except for a handful number of expensive brands.

Pretty much all camera systems have to deal with a few poorly implemented protocols in order to increase camera compatibility.

Also, this small config would not directly tweak the UI. I am not talking about adding an option to allow hiding the zoom element in the UI for some given camera. I am talking about an option to allow overriding the camera PTZ capabilities at backend level, which I assume the UI will later follow by polling backend for its capabilities (and all other UIs that rely on such backend call).

NickM-27 commented 3 months ago

it essentially is a UI toggle though, the information of what features each camera supports is not used for anything other than showing buttons in the UI. The problem is this gets complicated pretty quickly as each features could be sent wrong, presets could be named wrong, etc. This would become an entire block of new config entries quite quickly unless we made a specific exception for zoom which wouldn't make sense.

IMO these extra buttons in this edge case do not hurt or really impact the experience at all, at worst it is a visual bug

felipecrs commented 3 months ago

Just to make sure we are on the same page, are we both talking about a config like this?

cameras:
  street:
    onvif:
      ignored_features:
        - zoom

I see no reason to allow ignoring more features other than zoom unless users request it.

felipecrs commented 3 months ago

it essentially is a UI toggle though, the information of what features each camera supports is not used for anything other than showing buttons in the UI.

Doesn't auto tracking changes its behavior based on whether the camera has this ability or not? This would not be an UI thing.

NickM-27 commented 3 months ago

the problem is, as you pointed out, cameras often have bugs with onvif. So if we add a config for ignoring features that the camera says it supports, then a user may also say that their camera doesn't support naming presets but they want to name them. We would have the same reasons for not wanting to support that but it would be selective at that point and our logic would not really hold up. The question becomes where do you draw the line on camera bugs that you do and don't fix.

I don't see this as a real problem and we haven't had many reports or complaints about this. Perhaps in the future the opinion will change, but for now I believe this is working as intended.

Doesn't auto tracking changes its behavior based on whether the camera has this ability or not? This would not be an UI thing.

that is only done as a check, those attributes are not stored anywhere in the backend.

felipecrs commented 3 months ago

We would have the same reasons for not wanting to support that but it would be selective at that point and our logic would not really hold up. The question becomes where do you draw the line on camera bugs that you do and don't fix.

If users indeed have such problem about naming presets, it would be nice if Frigate could "fix" it too. Anything that software can do to avoid e-waste is good in my opinion.

What about highlighting this as a best effort, supported by community only, in the documentation? I.e. Frigate team shouldn't worry about implementing or fixing any features of this kind (if they once do, it's not because it was in their scope, but by the good of their hearts).

I would be very willing to implement this "feature" myself, for example.