Skip to content
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

FEAT: Remove best.jpg endpoint in favor of latest.jpg enpoint which uses snapshot from most recent event #2833

Closed

Conversation

NickM-27
Copy link
Collaborator

@NickM-27 NickM-27 commented Feb 19, 2022

Description

This PR is an implementation as described in #2799 to replace the best.jpg endpoint.

The new endpoint is GET /api/<camera_name>/<object_name>/latest.jpg This endpoint has feature parity with the old best.jpg endpoint and also includes the ability to pass any as the object_name and it will return the snapshot from the latest event of any type.

Open Questions

  • Currently the docs entry for best.jpg was removed but the API was left in place. Unsure if this should stay that way (perhaps redirect to the new API?) for a release and it can be considered deprecated? I am not sure if many users utilize this endpoint other than frigate itself.
  • Maybe should reverse the order of the two latest.jpg endpoints in the docs?

@NickM-27
Copy link
Collaborator Author

I have built locally and been testing this change on my main setup. It works great most of the time, but sometimes the crop=1 option which uses the box from the event. Seems to work great for objects that move toward the camera. But cars that drive by or move back and forth in the frame during the event end up getting cropped where they are only partially seen.

@blakeblackshear is there any difference with how the cords of box from the event is calculated compared to the box of the old best type?

@NickM-27
Copy link
Collaborator Author

Okay after some more looking, seems like after an event is ended the box property of the event is the last box that was drawn before the object stopped being detected. And since the snapshot is not necessarily the last frame it can be incorrect. Is there any way to circumvent this with existing data or should I work on potentially adding another property to event that is the snapshot_box.

Just to reiterate, this only affects images that are using the crop option

@NickM-27
Copy link
Collaborator Author

For some additional context, here is the difference:

This is the snapshot with bounding box (from events tab)

snapshot-1645472474 332064-oeopvl

This is the clean snapshot with a box drawn on it with the box coords saved in the DB (I added some logic to my API to draw a box on it)

image

@NickM-27
Copy link
Collaborator Author

NickM-27 commented Feb 21, 2022

Not sure if my fix is good enough to be mergeable, but I added best_box when the thumbnail is updated

self.thumbnail_data = {
    "frame_time": obj_data["frame_time"],
    "box": obj_data["box"],
    "area": obj_data["area"],
    "region": obj_data["region"],
    "score": obj_data["score"],
}
self.obj_data["best_box"] = obj_data["box"]
thumb_update = True

and made it so when the end fun is called it will tell the to_dict fun to use best_box instead of the standard box so that the thumbnail box is saved in the DB. From what I can tell, once the event is over the box is not used for anything.

    def to_dict(self, include_thumbnail: bool = False, end_frame: bool = False):
        snapshot_time = (
            self.thumbnail_data["frame_time"]
            if not self.thumbnail_data is None
            else 0.0
        )
        event = {
            "id": self.obj_data["id"],
            "camera": self.camera,
            "frame_time": self.obj_data["frame_time"],
            "snapshot_time": snapshot_time,
            "label": self.obj_data["label"],
            "top_score": self.top_score,
            "false_positive": self.false_positive,
            "start_time": self.obj_data["start_time"],
            "end_time": self.obj_data.get("end_time", None),
            "score": self.obj_data["score"],
            "box": self.obj_data.get("best_box", self.obj_data["box"]) if end_frame else self.obj_data["box"],
            "area": self.obj_data["area"],
            "region": self.obj_data["region"],
            "stationary": self.obj_data["motionless_count"]
            > self.camera_config.detect.stationary.threshold,
            "motionless_count": self.obj_data["motionless_count"],
            "position_changes": self.obj_data["position_changes"],
            "current_zones": self.current_zones.copy(),
            "entered_zones": self.entered_zones.copy(),
            "has_clip": self.has_clip,
            "has_snapshot": self.has_snapshot,
        }

        if include_thumbnail:
            event["thumbnail"] = base64.b64encode(self.get_thumbnail()).decode("utf-8")

        return event

@NickM-27
Copy link
Collaborator Author

After latest changes things are looking really good.

Screen Shot 2022-02-22 at 6 01 54 PM

Will continue testing to ensure no issues occur. Still thinking on best way to handle existing best endpoint (deprecate or just remove with chance of breaking change)

@NickM-27
Copy link
Collaborator Author

@blakeblackshear I think this PR is ready for review.

I have been testing for 5 days now and it has been working great to retrieve the snapshots and the crop options works well too. I am not sure how you feel about the "best_box" change I made to get cropping working, I think it isn't super clean but it also isn't too bad and works well. I'm not sure if you had plans to also remove some code from the frames_processor.best, I would be happy to look into it or maybe you would like to yourself, in any case perhaps that could go into its own PR?

If you want to give this one more time I completely understand, just thought I would reach out to see what you are thinking on this. Thank you! 😃

@blakeblackshear
Copy link
Owner

I think the best thing to do here is leverage the snapshot.jpg and thumbnail.jpg endpoints instead. All you have to do is query for the latest event id (in progress is fine too) and then call the existing functions to handle in progress events. The requests would end up being:

/api/<camera>/<object>/snapshot.jpg
/api/<camera>/<object>/thumbnail.jpg

I would recommend opening a new PR rather than updating this branch since there are some artifacts from all the back merging.

@NickM-27 NickM-27 closed this Mar 10, 2022
@NickM-27 NickM-27 deleted the better-object-jpg branch March 10, 2022 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants