Skip to content

Conversation

@skalthoff
Copy link

This pull request enhances error handling and robustness in the benchmark function of jellybench_py/core.py. The main adjustment ensures dictionary keys are safely accessed, preventing runtime errors caused by missing keys.

Previously, encountering missing keys (speed or rss_kb) in the max_pass_run_data dictionary triggered a KeyError, interrupting execution:

KeyError: 'speed'

Improvements:

  • Updated dictionary key access within the benchmark function
  • Implemented the get method with appropriate default values (0.0 for speed, and 0 for rss_kb) to gracefully handle missing keys.

These changes isolate the fix within the affected function, enhancing code stability without side effects elsewhere.

@BotBlake BotBlake self-requested a review May 18, 2025 11:15
Copy link
Owner

@BotBlake BotBlake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we only talked about this PR in private until now, let me comment on GitHub as well.
Implementation-wise it seems to do the same as #87
However I am not a fan of that implementation approach.

Specifically I want to understand WHY these values could be missing.
Since these values are IMPORTANT for the test results, I personaly do not think it is a good idea to have them be anything BUT the values provided directly from ffmpeg.

In my oppinion a test where ANY ffmpeg process does not return all requied values should be marked as FAILED, with a failure reason that explains the situation. (e.g. "missing_values")

Perhaps change the PR as a Draft if you intend to work on this.
Otherwise I suggest closing it for the time being and re-open it when necessary.

@skalthoff skalthoff closed this May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants