-
Notifications
You must be signed in to change notification settings - Fork 12
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
OAK-D support H264 and H265 video encoding and gzip detph data #993
Conversation
osgar/drivers/oak_camera.py
Outdated
class OakCamera: | ||
def __init__(self, config, bus): | ||
self.input_thread = Thread(target=self.run_input, daemon=True) | ||
self.bus = bus | ||
|
||
self.bus.register('depth', 'color', 'orientation_list', 'detections', | ||
self.bus.register('depth:gz', 'color', 'orientation_list', 'detections', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change increases the load on the cpu. In my opinion, this option (:gz
) must be optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about it? Realsense also uses :gz
version - I mean it will surely slow down CPU, but save time on storage??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p.s. it would be interesting/nice, to do this packing already on OAK-D side, but then you spend CPU time unpacking if you use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree let this be the default setting. However, in one application in particular we are hitting a cpu performance limit and this change will probably make our work more difficult.
Thus, if possible, I would be happy for the option to turn this off.
Yes, the situation is the same with the RS camera, but we don't use it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks - if we agree that default should be gzipped and you already have application which would reach the limit, then let's add the on/off option.
Please add some config parameter e.g. |
I hope this extra flag will work for you (if not please let me know). I tried also H265 and it also seems to work, but lidarview etc will be tricky. Thanks & merging |
before:
after