fix(logging): incorrect FPS logging #10921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
👋 Hello TNeutron, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
TNeutron
changed the title
Previously, last_frame was only updated once at the beginning of stream_handler, leading to incorrect FPS and avg_frame_time computation. This commit ensures last_frame is updated on each iteration after last FPS computation, resulting in accurate FPS logging. Fixes espressif#10920
|
Hi @TNeutron . I see the problem, but what you suggest is not the solution. int64_t frame_time = fr_end - last_frame;
last_frame = fr_end;
//... |
@me-no-dev Please let me know if you'd like me to edit the commit. |
|
Yes please :) revert your change and add the new one and we will merge it |
|
The average frame rate looks a bit weird. Like 10 times lower than it should be... I wonder why |
This reverts commit 0bb7b95.
Corrected and tested change in FPS computation, suggested by @me-no-dev and found working with correct numbers. Previously, last_frame was only updated once at the beginning of stream_handler, leading to incorrect FPS and avg_frame_time computation. This commit ensures last_frame is updated on each iteration after last FPS computation, resulting in accurate FPS logging. Fixes espressif#10920
|
Dear @me-no-dev Both FPS and Avg FPS is correct now. I'm still learning my way around git. Again, I appreciate your patience and kind response :) Looking forward to the merge 😅 |
me-no-dev
approved these changes
Jan 31, 2025
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
The code provides fixes incorrect FPS calculation issue. The last_frame variable needs to be updated on each iteration, Immediately after computing the FPS of previous.
Before this commit - the last frame was being updated only once at the beginning of stream_handler function. The fps and avg_frame_time computation were completely wrong. This commit fixes the computation.
This commit - updates last_frame variable on each iteration. results is correct fps calculation.
Tests scenarios
I have tested my Pull Request on latest Arduino-esp32 core with ESP32-CAM (CAMERA_MODEL_AI_THINKER) with Core Debug Level - Verbose
Related links
Please provide links to related issue, PRs etc.
(eg. Closes #number of issue)
Closes 1 issue ( #10920 )