Skip to content
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

Fix rv.commands.clearSession() caching issue #698

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bernie-laberge
Copy link
Contributor

@bernie-laberge bernie-laberge commented Feb 21, 2025

Fix rv.commands.clearSession() caching issue

Linked issues

NA

Describe the reason for the change.

Problem:
In some situations, invoking the rv.commands.clearSession() would leave with the caching turned off for the remainder of the RV process lifetime which was impacting playback performances for the remainder of the RV process lifetime.
Note that the caching=off would not be persisted. So at the next RV relaunch, the caching would be enabled.

This problem can be reproduced when importing an OTIO into RV.

Before the OTIO import, caching is on:
rv_cache_is_initially_on_good

After the OTIO import, the media caching would be effectively disabled:
rv_cache_is_turned_off_after_otio_import

Summarize your change.

Cause:
This was caused by the rv.commands.clearSession() which momentarily turn off caching to empty its content.
But the original caching state is never restored until the user relaunches RV.

Fix:
Now restoring the original caching state in Session::clear().

Describe what you have tested and on which operating system.

Successfully tested on macOS. This issue is not OS specific.

Add a list of changes, and note any that might need special attention during the review.

If possible, provide screenshots.

Copy link
Contributor

@eloisebrosseau eloisebrosseau left a comment

Choose a reason for hiding this comment

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

Nice catch, thank you!

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