-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Frame queue per stream, not per extension #12575
Conversation
f49d17f
to
9bc101c
Compare
src/source.h
Outdated
|
||
class LRS_EXTENSION_API frame_source | ||
{ | ||
public: | ||
frame_source(uint32_t max_publish_list_size = 16); | ||
//using stream_uid = std::pair< rs2_stream, int >; // Stream type and index |
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.
Remove
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.
Done
src/source.cpp
Outdated
return _archive[RS2_EXTENSION_VIDEO_FRAME]->begin_callback(); | ||
// return _archive[RS2_EXTENSION_DEPTH_FRAME]->begin_callback(); | ||
if( id.second >= RS2_EXTENSION_COUNT ) | ||
id.first = RS2_STREAM_COUNT; // For added extensions like GPU accelerated frames |
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.
Needed?
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.
Yes needed.
When adding an extension we need to create the archive because add_extension
is templated with the archive type. At that stage we don't know the stream type that will be used, but cannot keep the type for later allocation.
When using the allocated archive we need to find
it with the same key.
src/source.cpp
Outdated
if( it == _archive.end() ) | ||
{ | ||
create_archive( id ); | ||
it = _archive.find( id ); // Now will successfully find |
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.
Verify if we can optimize
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.
Optimized. create_archive
now returns iterator to new archive.
src/source.cpp
Outdated
throw wrong_api_call_sequence_exception( "Requested frame type is not supported!" ); | ||
{ | ||
create_archive( id ); | ||
it = _archive.find( id ); // Now will successfully find |
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.
Same
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.
Done
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.
Optimized
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.
Small comments
…terator from create_archive
Currently frame archives are created per extension type. There are several streams that use
RS2_EXTENSION_VIDEO_FRAME
and the queue might get full, especially if a syncher is involved.This PR creates archives also based on the stream type, so IR1+2 still share a queue but they come together and don't have to wait in the syncher.
Tracked on [RSDEV-599]