Error on playing paused bag - #9887
Conversation
|
|
||
| auto capture_time = std::chrono::high_resolution_clock::now() - m_capture_time_base; | ||
|
|
||
| if (m_record_total_pause_duration > std::chrono::nanoseconds::zero()) |
There was a problem hiding this comment.
Just for protection and to emphasize that we only deduct the pause duration if it's non zero, probably can be removed if we like..
| } | ||
| else | ||
| { | ||
| m_record_total_pause_duration += now - m_capture_time_base; |
There was a problem hiding this comment.
Can't we just do all of this in one line?
m_record_total_pause_duration += now - std::max( m_capture_time_base, m_time_of_pause );
There was a problem hiding this comment.
Note that we are inside the else of if ( m_capture_time_base < m_time_of_pause )
Which means that m_capture_time_base is >= m_time_of_pause , so we don't need std::max
That's the idea, we only need the offset, I tried to explain it in the comments above
| record_with_pause( file_name, iterations=1, pause_delay=3, resume_delay=2 ) | ||
| pipeline = rs.pipeline() | ||
| device_playback = playback( pipeline, file_name, signal_on_stop ) | ||
| verify_stop_when_eof( timeout=15 ) |
There was a problem hiding this comment.
Why 15?
3+2+3 = 8... we're allowing way too much for playback, no?
| record_with_pause( file_name, iterations=2, pause_delay=0, resume_delay=2 ) | ||
| pipeline = rs.pipeline() | ||
| device_playback = playback( pipeline, file_name, signal_on_stop ) | ||
| verify_stop_when_eof( timeout=20 ) |
There was a problem hiding this comment.
Shouldn't the playback finish within 10?
Any way we can set the expected time automatically, given the parameters from record_with_pause??
There was a problem hiding this comment.
I wanted to test that the BUG was fixed and it'[s not coming back.
I didn't want to test the record and playback performance as I now it's not hard realtime and we see playback test fail once in a few days..
I will add the calculated timeout + an extra time and lets see how it goes..
|
By the way, the unit-test does not cover corner cases, but we'll leave that for another test later:
|
I know but when I get a task of fixing a BUG, I fix the BUG and add a regression test that make sure this bug is not coming back ( prevention action ), I don't test the whole system... As you said, we can add a task of expending the test.. |
| stop_detected = True | ||
|
|
||
| def calc_timeout ( iterations, pause_delay): | ||
| return iterations * (pause_delay + STREAMING_DURATION) + TIMEOUT_BUFFER |
There was a problem hiding this comment.
Is this right? Why isn't the resume delay in there?
There was a problem hiding this comment.
The delay between pause and resume should not increase playback time as I understand it.
We are not recording at that time.
Right?
There was a problem hiding this comment.
Ahh, correct! I'll add a comment to that effect, thx!
(cherry picked from commit f8fada3)
|
@maloel The issue was resolved by the latest SDK 2.50. Could you please try and update? Thanks! |
Fix issue #4249
When the playback pause action occurs before the first frame arrive the timestamp creation was corrupted.
A sleep between
pipeline.start()andrecorder.pause()was a walkaround.This PR fix the corrupted timestamp creation
Note that the problem was at the recorder side so a new bag will be needed to resolve this playback issue.
Tracked on [DSO-14342]