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

Hint that metadata support requires OS level settings/changes #5669

Closed
wants to merge 1 commit into from

Conversation

radfordi
Copy link
Contributor

Warn users that try and fail to access frame metadata that they may be able to enable support for frame metadata by making OS level changes. It took me quite a long time to figure this out, and other users continue to run into this problem.

@@ -120,7 +120,7 @@ namespace librealsense
auto s = reinterpret_cast<const S*>(((const uint8_t*)frm.additional_data.metadata_blob.data()) + _offset);

if (!is_attribute_valid(s))
throw invalid_value_exception("metadata not available");
throw invalid_value_exception("metadata not available (to enable metadata support in your OS, see the documentation)");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radfordi , there is a nuance - the metadata is not guaranteed for all frames even if it is enabled on OS level as it is the FW that controls it. Hence the absence of metadata attribute shouldn't be automatically attributed to OS.
I'm really looking how to make this message informative yet concise as it is expected that this will be fired more than once per streaming session. Do you have any suggestion?

Copy link
Contributor Author

@radfordi radfordi Jan 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to make this message most informative is to detect if support is present in the Linux kernel, or if the appropriate registry magic has been done on Windows, and to change the message based on the result. In the meantime, I think my suggestion in this patch is a significant improvement to the status quo.

It's a large roadblock to expect user's to know via some magic, since it's not in CAPITAL LETTERS at to top of the README, nor even in the README at all, that you'll likely have to patch your kernel to get auto-exposure times. There's no warning in the realsense-viewer that you are using an unpatched kernel. There is zero feedback. It was a hair pulling experience for me to get exposure times, and it's clearly the case for many other users. In the end I had to talk to someone on the firmware team to find out that I had to upgrade my firmware and patch my kernel before this basic functional would work. Providing feedback to users in the first message they will see is the least we can do.

@ev-mp
Copy link
Collaborator

ev-mp commented Jul 22, 2020

Closed

@ev-mp ev-mp closed this Jul 22, 2020
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