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

Revert "Disable cameras after fetching images, projection matrix (#2881)" #3064

Merged
merged 1 commit into from
Oct 8, 2020

Conversation

rajat2004
Copy link
Contributor

Running the Image API continuously seems to trigger exceptions

This reverts commit ba4be15.

…rosoft#2881)"

Running the Image API continuously seems to trigger exceptions

This reverts commit ba4be15
@saihv
Copy link
Contributor

saihv commented Oct 7, 2020

Any thoughts on why this might be causing image capture issues? Could it be because the image API is accessing the camera again just as it's being disabled? (Most Unreal operations might be bound to the scene tick rate)

@rajat2004
Copy link
Contributor Author

Not sure, most likely along what you're thinking, it could be marking it as "Disabled", but some stuff happens later on in the next tick and sometimes things don't work out well. Comment here indicates that this does cause crashes
I'm not well-versed enough in UE to say anything for sure though

@rajat2004
Copy link
Contributor Author

I've changed it from a draft PR since it has been tested to fix the issue by at least 1 user, though more testing would be good
Should we instead add a simDisableCamera API as was done in the original PR #2465?

@m1baldwin
Copy link

m1baldwin commented Oct 7, 2020

Just wondering, is the net effect of the original PR improving the frame rate to toggle the camera device off in between image requests? I suppose this would mean the camera is being activated/deactivated too rapidly - as it is re-activated when a request comes in - which the comment @rajat2004 pointed out in line 457 might be causing unreal to crash due to unnecessary calls to Activate()?

@saihv
Copy link
Contributor

saihv commented Oct 8, 2020

I was able to repro both the issue and the fix from this PR. Merging this as an immediate fix, we can continue discussion about how best to re-implement the disable feature.

@saihv saihv merged commit d59ceb7 into microsoft:master Oct 8, 2020
@rajat2004 rajat2004 deleted the revert-disable-cameras branch October 8, 2020 03:38
@rajat2004
Copy link
Contributor Author

Great, thanks for testing and merging!
@m1baldwin Yeah, not exactly sure of the reason, but seems like enabling and disabling cameras isn't a completely synchronous task, and activating the camera while not completely deactivated could be causing the crash. This was more apparent in #3018 due to the higher FPS

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.

3 participants