-
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
Test playback behavior with syncer at EOF #9668
Conversation
… be used interchangeably
global syncer, playback_status | ||
f = syncer.poll_for_frame() | ||
if playback_status is not None: | ||
countdown = 50 # 5 seconds |
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.
Why not using a timer class.
Set it to 5 seconds and check has_expired()
It will be more elegant IMO
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.
It's easier for me to implement simple code like this without remembering APIs :)
elif not depth: | ||
color = rs.video_frame( f ) | ||
else: | ||
color = None | ||
test.info( "actual color", color ) | ||
test.check_equal( color_frame is None, not color ) | ||
if color_frame is not None and color: | ||
test.check_equal( color.get_frame_number(), color_frame ) |
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.
Oh, I just realized that color_frame
is actually color_frame_number
I didn't understand why we compare a frame number to a frame.
consider adding the arguments "_number" suffix
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.
Yeah it's a mess. If I change it I have to change lots of lines in alll 3 testcases. I'll do it separately.
unit-tests/syncer/test-ts-eof.py
Outdated
|
||
sw.generate_depth_and_color( frame_number = 0, timestamp = 0 ) | ||
sw.expect( depth_frame = 0 ) # syncer doesn't know about color yet | ||
sw.expect( color_frame = 0, nothing_else = True ) # less than next expected of D |
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.
less than next expected of D
,
- D = depth?
- What do you mean by less than next expected. I didn't understand the comment
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.
You need to understand the syncer: every stream has a time at which we expect the next frame to arrive. This is called the "next expected".
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 know what next expected means :)
I meant that a comment like "Since the next expected time for a depth frame is less than the time the color frame arrive, the syncer will not wait for it and output the color frame"
Will be more verbose without knowing the inside internal parameters of the syncer.
test.finish() | ||
# | ||
############################################################################################# | ||
test.print_results_and_exit() |
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.
Great UT.
It really help to understand the "under the hood" of the syncer
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.
Great UT 🥇
Added improvements to Python wrapper usability:
frame_queue
-likewait_for_frame
etc. tosyncer
so they can be used interchangeablyAlso:
rs-convert
to easily see the frame content of a rosbagTracked on [LRS-172]