Handle spaces in labels

[uvjustin]

Also use camera_name instead of camera in all routes for consistency.

Something like this should work for #3678, but I haven't tested this yet.

[hawkeye217]

Thanks for this! I was going to create a PR next week when my schedule freed up a bit, but I'm glad you beat me to it. I love the open source community!

[uvjustin]

@hawkeye217 Can you test it on your end? I don't have any events with those labels, and more importantly I have a hard time building the frontend as my 7 year old laptop doesn't seem to be able to run the dev container.

[hawkeye217]

@uvjustin I don't have a full dev environment set up either, but you can actually just build the frontend and connect it to an existing Frigate instance.

I did that and copied in your modified http.py to my existing Frigate instance (I know, quite a hack, but it was faster than building everything just to test the change). And I got the same 400 error.

I think the issue is with the nginx configuration inside the container. Looking at the nginx docs and this issue, it appears that the proxy_pass parameter expects a properly escaped uri when variables are used.

@blakeblackshear made this commit to nginx.conf, and I'm guessing it was probably to cache images. I removed the entire section, lines 175-183 from nginx.conf and image urls with parameters are still working, likely because the section below it (lines 185-196) is correctly proxying the api endpoint. The 400 error is also gone and images with spaces in the label display correctly in the frontend now.

I'm no nginx expert, so maybe there's a better solution. I think there's caching with images to consider as well. Hope this helps!

[hawkeye217]

I also realized the frontend isn't required to reproduce the bug - just making a request to the frigate http api endpoint for an object type that has a space in the label will cause the nginx error. For example:

http://192.168.1.100:5000/api/frontcamera/sports%20ball/best.jpg?crop=1&h=150

will return 400 from nginx.

[NickM-27]

for now you could rename objects in the labelmap to have the _

[hawkeye217]

for now you could rename objects in the labelmap to have the _

For sure. That would work for objects moving forward. But anything already in the database for the old label wouldn't show up in the event list.

Not a big deal for me, but I'm thinking ahead to Frigate+ and any labels that have spaces.

[NickM-27]

But anything already in the database for the old label wouldn't show up in the event list.

They show up, the db doesn't clear out old events like that, same thing as to why #3184 can create custom labels without worrying about that. And Frigate+ could similarly not use spaces and use _ instead

In any case, it should be fixed in the longer term for sure.

[hawkeye217]

But anything already in the database for the old label wouldn't show up in the event list.

They show up, the db doesn't clear out old events like that, same thing as to why #3184 can create custom labels without worrying about that. And Frigate+ could similarly not use spaces and use _ instead

In any case, it should be fixed in the longer term for sure.

Maybe I was misunderstanding. I updated my labelmap and my config to change "sports ball" to "ball", and the events that were categorized under "sports ball" no longer show up in my UI after restart. Of course they're still in the database, because I can switch everything back in the config, restart, and they all show up in the UI again.

Not a common use case, though.

And I didn't have the issue on 0.10.1. I only noticed it when upgrading to 0.11 pre.

Other than this and the jellyfin issue, 0.11 has been great for me!

[NickM-27]

Frigate+ actually already does use _ for example in the label license_plate.

and the events that were categorized under "sports ball" no longer show up in my UI after restart.

Interesting, the UI should be a direct reflection of the db so that doesn't seem to make sense 🤔
Hopefully there's no cleanup behavior I am unaware of that would affect my PR, but it is staying in the DB so that doesn't make sense.

Seems like Nginx doesn't make this particular case easy assuming that the variables are needed.

[uvjustin]

I could build the frontend outside the container, but given that I don't even have events with the offending labels to easily test on, I'd rather not clutter up this old computer (I don't use JS much and don't even have npm on the computer). Anyway, thanks for taking the time to try it out.
The NGINX issue should be easily worked around with a rewrite. I just pushed a commit that should fix this.

[uvjustin]

I noticed @NickM-27 opened up #3708 which renders _ as spaces in the frontend. In that case, rending a label with _ and a label with space will look just as pretty, and perhaps just converting spaces to _ when importing the coco labels is the easier solution. But since the old stored events will still have the old label name (with spaces), these names would have to be updated in migrations.
Let's see what Nick and Blake prefer to do.

[NickM-27]

To be clear, my change was just for the frontend readability and wasn't to address this. I don't know enough about nginx to say what the preferred approach is 👍

Either solution seems viable to me

[blakeblackshear]

Tested this by changing "person" to "per son" and everything appears to be working correctly.

[hawkeye217]

I can also confirm the updated nginx.conf is working as expected. Thanks @uvjustin!