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

Ask user during find corner at the end to validate the data #90

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ANaaim
Copy link
Contributor

@ANaaim ANaaim commented Apr 9, 2024

Could be good if you test also (it work fine for me)

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 9, 2024

linked to #89

@davidpagnon
Copy link
Collaborator

davidpagnon commented Apr 9, 2024

Thank you Alex for this pull request!
It works great, however I ran into two small issues:

  • I had to explicitly add import tkinter.messagebox to the file, otherwise I ran into an error
  • There is a tiny tkinter window that is floating around, and if I try to close it it causes python to exit. Do you have it on your side?
  • And a minor additional thing: the window is not currently triggered if I type H (hidden last point) instead of if I click. If would be nice if that case was taken into account, too

image

@davidpagnon
Copy link
Collaborator

Last thing @ANaaim, do you think you could pop that window both when clicking the intrinsics and the extrinsics? (apologies if you already did)

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 12, 2024

I will have a look at it on Monday.

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 15, 2024

@davidpagnon (some answer in your message)
Thank you Alex for this pull request!
It works great, however I ran into two small issues:

  • I had to explicitly add import tkinter.messagebox to the file, otherwise I ran into an error
    • I do not understand why because when used I definitely use as the following :
msg_box = tkinter.messagebox.askquestion(
                            "Validate Calibration", "Are you satisfied with point positioning ? ", icon="warning"
                        )
  • There is a tiny tkinter window that is floating around, and if I try to close it it causes python to exit. Do you have it on your side?
    - Yes I will remove it. It should not be difficult.
  • And a minor additional thing: the window is not currently triggered if I type H (hidden last point) instead of if I click. If would be nice if that case was taken into account, too
    • Would it be possible to switch all this bit below in on_click instead of on_key (as it is linked to adding point it seems more logical)
        if event.key == 'h':
            # If 'h', indicates that one of the objp is not visible on image
            # Displays it in red on 3D plot
            if len(objp) != 0  and 'ax_3d' in globals():
                count = [0 if 'count' not in globals() else count+1][0]
                if 'events' not in globals():
                    # retrieve first objp_confirmed_notok and plot 3D
                    events = [event]
                    objp_confirmed_notok = objp[count]
                    ax_3d.scatter(*objp_confirmed_notok, marker='o', color='r')
                    fig_3d.canvas.draw()
                elif count == len(objp)-1:
                    # if all objp have been clicked or indicated as not visible, close all
                    objp_confirmed = np.array([[objp[count]] if 'objp_confirmed' not in globals() else objp_confirmed+[objp[count]]][0])[:-1]
                    imgp_confirmed = np.array(np.expand_dims(scat.get_offsets(), axis=1), np.float32) 
                    plt.close('all')
                    for var_to_delete in ['events', 'count', 'scat', 'fig_3d', 'ax_3d', 'objp_confirmed_notok']:
                        if var_to_delete in globals():
                            del globals()[var_to_delete]
                else:
                    # retrieve other objp_confirmed_notok and plot 3D
                    events.append(event)
                    objp_confirmed_notok = objp[count]
                    ax_3d.scatter(*objp_confirmed_notok, marker='o', color='r')
                    fig_3d.canvas.draw()
            else:
                pass

For the last bit, maybe the function on_key and on_click should be renamed menu_event and adding_point_event (or something like this)

@davidpagnon
Copy link
Collaborator

davidpagnon commented Apr 15, 2024

  • do not understand why because when used I definitely use as the following :
msg_box = tkinter.messagebox.askquestion(
                            "Validate Calibration", "Are you satisfied with point positioning ? ", icon="warning"
                        )

You are right, this is strange, I could have made a mistake.


For the last bit, maybe the function on_key and on_click should be renamed menu_event and adding_point_event (or something like this)

Sure, that sounds coherent! I'm all up for something clearer (as long as it still works :) )

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 15, 2024

Work with intrinsic as well as it is using the same function.

@davidpagnon
Copy link
Collaborator

davidpagnon commented Apr 15, 2024

Cool, the messagebox is now closing, and it shows both for intrinsics and for extrinsics! A couple last remarks:

  • I don't quite understand why, but I do need to import tkinter.messagebox or it throws an error at me.
    However, I notice that the following imports are not useful anymore:
    import tkinter
    from contextlib import contextmanager,redirect_stderr,redirect_stdout
    from os import devnull

  • There is an unnecessary print(count) L978

  • The lines 524+ are not needed, as discussed in this issue

     ret, C, S, D, K, R, T = calibrate_intrinsics(calib_dir, intrinsics_config_dict)
    
    # calculate extrinsics
    if calculate_extrinsics:
       logging.info(f'\nCalculating extrinsic parameters...')

    (and L689, imgp == [] must be replaced with len(imgp) == 0).

  • I added objpoints = np.array(objpoints) L605, otherwise I've got an error when clicking intrinsic parameters:

    error: OpenCV(4.9.0) :-1: error: (-5:Bad argument) in function 'calibrateCamera'
    > Overload resolution failed:
    >  - Can't parse 'objectPoints'. Sequence item with index 0 has a wrong type
    >  - Can't parse 'objectPoints'. Sequence item with index 0 has a wrong type
  • Lastly, it seems like pressing 'H' does not work anymore: the hidden objpoint is removed from the list and then cv2.calibrationCamera() does not get the expected size for objpoints. I haven't looked into solving this yet.

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 16, 2024

I tested the h button with board, it seemed to work fine. Is there different stuff associated with using a scene that could result in this behavior ?

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 16, 2024

Not sure to understand where it should be put for this :

I added objpoints = np.array(objpoints) L605, otherwise I've got an error when clicking intrinsic parameters:
This :

                imgp_confirmed = findCorners(img_path, intrinsics_corners_nb, objp=objp, show=show_detection_intrinsics)
                if isinstance(imgp_confirmed, np.ndarray):
                    imgpoints.append(imgp_confirmed)
                    objpoints.append(objp)

Should become this ? :

                imgp_confirmed = findCorners(img_path, intrinsics_corners_nb, objp=objp, show=show_detection_intrinsics)
                if isinstance(imgp_confirmed, np.ndarray):
                    objpoints = np.array(objpoints)
                    imgpoints.append(imgp_confirmed)
                    objpoints.append(objp)

@davidpagnon
Copy link
Collaborator

Oh yes sorry the lines have shifted:

        # calculate intrinsics
        img = cv2.imread(str(img_path))
        objpoints = np.array(objpoints) # <-- here (L602)
        ret_cam, mtx, dist, rvecs, tvecs = cv2.calibrateCamera(objpoints, imgpoints, img.shape[1::-1], 
                                    None, None, flags=(cv2.CALIB_FIX_K3 + cv2.CALIB_FIX_PRINCIPAL_POINT))

I still have that issue with 'H', do you not have it at all? The error does not happen right away, but after I've done all the clicking for all images for a given camera.

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 17, 2024

Is the error in calibration intrinsic only ?

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 17, 2024

Ithink that the problem come from the fact that during a intrinsic calibration it create a list of associated objpoint and imgpoint that might have difference in size... as a result it might not work. But for the intrinsics calibration for me it should not be possible to do a calibration without seeing the full board...

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 17, 2024

line: 607 objpoints = np.array(objpoints)

Here all the objpoints element of the list should have the same dimension if we want to transform it to a np array. For me the H as sense only during the extrinsics calibration. (We should even deactivate H during intrinsics)

@davidpagnon
Copy link
Collaborator

davidpagnon commented Apr 17, 2024

Why should we deactivate it? If we don't have that many checkerboard images, and for one of them some corners are out of the frame (like in this one), it would be nice to keep the option of declaring that a point is not seen, wouldn't it?

image

@ANaaim
Copy link
Contributor Author

ANaaim commented Apr 17, 2024

Ok but in that case the problem is in the following function

ret_cam, mtx, dist, rvecs, tvecs = cv2.calibrateCamera(objpoints, imgpoints, img.shape[1::-1],
                                                               None, None,
                                                               flags=(cv2.CALIB_FIX_K3 + cv2.CALIB_FIX_PRINCIPAL_POINT))

Can it take objpoints and imgpoints that are a list of numpy array that are not the same shape ? Because each associated obj_confirmed and img_confirmed are correctly associated with the same number of point.

                    imgpoints.append(imgp_confirmed)
                    objpoints.append(objp_confirmed)

@davidpagnon
Copy link
Collaborator

No, they need to be the same shape.

I would need to go back to the code to remember the details, but the rationale for the way I did it was in The way I did it was in imgp_objp_visualizer_clicker:
If 'H' was typed for a certain index, imgp was not created, and the corresponding objp was removed from objpoints

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.

2 participants