[API/Alerts] top_score not parsed correctly?

SHU-red commented 1 week ago

Describe the bug

Yesterday i made use of your awesome and cleanly documented project → Thanks for that 🙏
Everything is up and my telegram notifications are working
I wanted to use the alerts - labels - min_score setting to only get notified above a certain score
I did set it to 75 and did not get any notification anymore, despite having many events with higher score than 75
In the telegram message text (which i did not change from default) is the correct label & score

I checked my /api/events endpoint and got (which shows a top-level top_score of "null", within the data section the correct score is shown

[
  {
    "box": null,
    "camera": "garage",
    "data":
      {
        "attributes": [],
        "box":
          [
            0.428125,
            0.1425925925925926,
            0.37864583333333335,
            0.8527777777777777,
          ],
        "region":
          [0.3104166666666667, 0.0, 0.6145833333333334, 1.0925925925925926],
        "score": 0.9731574058532715,
        "top_score": 0.9655845165252686,
        "type": "object",
      },
    "end_time": 1730320228.789495,
    "false_positive": null,
    "has_clip": true,
    "has_snapshot": true,
    "id": "1730320212.849335-3ck4p9",
    "label": "person",
    "plus_id": null,
    "retain_indefinitely": false,
    "start_time": 1730320207.849335,
    "sub_label": null,
    "thumbnail": "nonono",
    "top_score": null,
    "zones": [],
  },
]

I already read #54 which seems to be linked to this but does not mess with min_score
I already made sure at frigate that the top_score on top-level is deprecated: https://github.com/blakeblackshear/frigate/discussions/14696

Expected behavior

After setting min_score to 75 i shall still get notifications but only for images containing objects with higher score of 75

To Reproduce Steps to reproduce the behavior:

Versions:

Frigate NVR: 0.14.1-f4f3cfa
Frigate-Notify: 0.3.5

Frigate-Notify Config:

alerts:
  labels:
    min_score: 75

Additional context

Everything above is just guessed
If im wrong i can try to dig more deep into my specific problem but it feels like the (?lately?) done changed in frigate-api could lead to this inconsistency
BTW: Is there a way to set a score per label?

0x2142 commented 1 week ago

Hi there, Thanks for catching this. After a bit of digging, it looks like in #54 I fixed the issue with the top-level top_score being 0% - which was in response to changes made by the Frigate project. However, that was more-or-less only for the text added to notifications. The ability to filter events based on a minimum score wasn't implemented for a few months later - and unfortunately this filtering occurs in the code long before I re-map the score from the earlier fix.

So I'll have to make a few changes to get this addressed

SHU-red commented 1 week ago

Thank you very much I really appreciate your hard work

That said, I just wanted to mention that I would love to be able to set a separate min Score for each label

E.g.: Cats: 90% Person: 85%

Just because I think you have to touch this code anyways 😉

freefd commented 1 week ago

I just wanted to mention that I would love to be able to set a separate min Score for each label E.g.: Cats: 90% Person: 85%

Dear @SHU-red, kindly create a separate improvement request for this, as not to mix improvement and issue in the same thread.

In addition, about the score over sub label:

Frigate documentation and code show that score might appear in the /events topic: https://github.com/blakeblackshear/frigate/blob/dev/docs/docs/integrations/mqtt.md?plain=1#L63, https://github.com/blakeblackshear/frigate/blob/dev/web/src/types/event.ts#L19
At the same time, documentation for the /reviews topic didn't mention any score: https://github.com/blakeblackshear/frigate/blob/dev/docs/docs/integrations/mqtt.md?plain=1#L138

Dear @NickM-27, can you guide us here, please, if the sub labels of after block in Frigate event can have the score attribute? Thank you.

NickM-27 commented 1 week ago

The /events topic contains a tuple for that specific object and the score, the /reviews topic just has a list of sub labels that were detected during that review time period

freefd commented 1 week ago

@NickM-27, thanks a lot for the quick response! Does this mean that once Frigate-notify starts working with the /reviews topic instead of /events, the scores of sub labels will become unavailable and hence filtering on these sub label scores cannot be implemented after switching?

SHU-red commented 1 week ago

Dear @SHU-red, kindly create a separate improvement request for this, as not to mix improvement and issue in the same thread.

Fully aggree :point_right: #164

0x2142 commented 1 week ago

Hi @SHU-red - I added a quick fix for your original issue in the latest dev build. If you don't want to use the dev branch, and need the fix sooner - let me know & maybe I can throw together a quick release for this.

SHU-red commented 1 week ago

Damn u fast! Thank you!!!! Its not that urgent but i will immediately pull the latest :dev container and feel refreshed :laughing:

NickM-27 commented 1 week ago

Does this mean that once Frigate-notify starts working with the /reviews topic instead of /events, the scores of sub labels will become unavailable and hence filtering on these sub label scores cannot be implemented after switching?

correct, and personally I don't see a reason for a notification script to implement this, as realistically it should be up to the user to configure the software that creates the sub label to only create it if the confidence is high enough. Once frigate has the sub label, it should be trusted

freefd commented 1 week ago

Once frigate has the sub label, it should be trusted

Good point, ok. Thanks again.

@SHU-red, finally, sub labels are out of the scope of #164.

SHU-red commented 1 week ago

Hi @SHU-red - I added a quick fix for your original issue in the latest dev build. If you don't want to use the dev branch, and need the fix sooner - let me know & maybe I can throw together a quick release for this.

Not sure if this is related to your fix

{events}

panic: runtime error: invalid memory address or nil pointer dereference

[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x78ab84]

goroutine 1 [running]:

github.com/0x2142/frigate-notify/config.Config.validate({{{0x852f67, 0x6}}, 0xc0001a20f0, 0xc000005188, 0x0})

    /app/config/validate.go:120 +0x12c4

github.com/0x2142/frigate-notify/config.LoadConfig({0x0?, 0x91f72d?})

    /app/config/config.go:212 +0x405

main.main()

    /app/main.go:87 +0x11a9

0x2142 commented 1 week ago

@SHU-red - Yeah, I had a lot of other changes included in that build that caused your error. I just pushed another image - this error should be fixed for you as well.

SHU-red commented 1 week ago

OK now notifications seem to be popping up again But (and i absolutely do not care) the score for the label in the notification message shows 0% I absolutely dont care for a dev version

Thank you very much

0x2142 / frigate-notify

[API/Alerts] top_score not parsed correctly? #161