collin80 / SavvyCAN

QT based cross platform canbus tool
MIT License
1.04k stars 288 forks source link

Incrrect playback time, shifted by 1 row #844

Open michcior2 opened 2 months ago

michcior2 commented 2 months ago

When player is playing log file with "Use original frame timing from captured file", it incorrectly uses timestamps. Takes 1 row ahead. See screenshot. Left is the player, right is just log load. Same CSV file. No meter what is in the file. It is always like that. Linux, not verified on windows/mac

obraz

michcior2 commented 2 months ago

So, it looks like there is also a problem with speed. Too short time between frames, when there are very short time gaps before frames, the player has more then one frame to play in the one timer trigger event. The frame is then lost and time stamp gets shifted.

So a dirty fix: In the function void FramePlaybackObject::timerTriggered() disable "while" which suppose to handle this cases, simple "break" fixes but still the last frame from the log is not sent.

//qDebug() << playbackLastTimeStamp; //qDebug() << "El: " << elapsed; while (peekPosition(true) <= playbackLastTimeStamp) { updatePosition(true); break; }

michcior2 commented 2 months ago

Another dirty fix. The main problem of this bug, seams to get fixed when: Function: quint64 FramePlaybackObject::peekPosition(bool forward) { int peekCurrentPosition = currentPosition; if (forward) { if (peekCurrentPosition < (currentSeqItem->data.count())){ //peekCurrentPosition++; //still in same file so keep going } else //hit the end of the current file { return 0xFFFFFFFFFFFFFFFFull; } } else { if (peekCurrentPosition > 0) peekCurrentPosition--; else //hit the beginning of the current sequence { return 0xFFFFFFFFFFFFFFFFull; } } CANFrame *thisFrame = &currentSeqItem->data[peekCurrentPosition]; return thisFrame->timeStamp().microSeconds(); }

1) disable //peekCurrentPosition++; this fixes issue with playbeck time shifted by 1 row 2) remove "-1" from codition: f (peekCurrentPosition < (currentSeqItem->data.count())) this fixes not sending the last position from the loaded log file.

Obviously, these are dirty fixes and, since I have no idea about overall picture, they need to be wisely re-implemented by the authors.

collin80 commented 2 months ago

Yes, I've noticed that it seems like it works one off so it's a little bit messed up. Certainly I do need to get that fixed.