obsproject
UI: Add frontend event and output signal for replay buffer saving
Description
This adds a frontend event and a signal on the replay buffer's signal handler that trigger when the replay buffer is saving (i.e. when a save has been requested, but before it saves). The frontend event triggers the moment the frontend requests a save (either through hotkey or the button), whereas the signal triggers when the replay buffer actually starts saving (separated by the encoding delay).
N.B.: Allowing the save hotkey to trigger a frontend event required moving its registration from obs-ffmpeg (in the definition of the replay buffer output) to the UI (with all the frontend hotkeys for outputs). This actually constitutes the biggest change in this PR.
Motivation and Context
Motivation is mainly to bring replay buffer save to feature parity with other output events, which almost all have both "before" and "after" events. A specific usecase envisionned was to allow the dynamic changing of replay buffer saving path/filename rather than having one set-in-stone save formatting for the whole session, but many other usecases could be possible.
Moving over the hotkey registration also has the added benefit of moving its location next to the other replay buffer hotkeys in the settings UI, which creates a more consistent UX.
How Has This Been Tested?
This has been tested on Ubuntu 22.04. Changes to hotkey registration seem to not have affected its behaviour. Event and signal trigger correctly from hotkey and button.
Types of changes
- New feature (non-breaking change which adds functionality)
- Documentation (a change to documentation pages)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
the hotkey member isn't used anywhere in obs code as far as I can see, but the commit that put it there, 87a64e6 , states it does so to "Allows the ability to use it for more output types". So I preferred to not remove it in case it was used by some third party's output.
If you think that's mistaken, I'll gladly remove it.
The two parts ("add signal and event" and "move hotkey registration") are not technically dependent on each other.
However, if the hotkey isn't moved, saving by pressing the hotkey will not trigger the firing of the frontend event (unless you can access the frontend api from within ffmpeg-mux). Event will only be sent when pressing the UI button, which does not feel to me like wishable behaviour.
Would also like a @Warchamp7 review on the fact that the save replay buffer hotkey is now always available, even if replay buffer is disabled.
I am okay with this change
(from what I can see the failed checks are just a result of temporary changes to the CI when this was last pushed, not of the changes introduced by this PR)
I just wanted to selfishly poke over here and see what's up. It'd be nice to have this functionality for using plugins with the replay buffer in the main branch instead of having to build with these changes pulled or use custom build artifacts.
@gxalpha is the only concern with this change right now (other than merge conflicts) that the two commits for moving the hotkey should be squashed into one?
trying to bump this up again for equally selfish reasons. could we get this looked at again and ideally merged? as it stands right now, for many use cases we cannot use the official OBS release as a replacement for shadowplay, and using @Penwy's build works great, but has the problem that some anti cheat software blocks OBS from recording specific games (like Helldivers 2 for example) since the fingerprints don't match (at least I assume that's the reason, official builds work fine, but any fork seems to be blocked). Me and a few others I know of would be very grateful to see this merged.
@gxalpha @RytoEX @Warchamp7 what is currently blocking this? I'd be happy to help make any changes required to get this merged.
It seems like a very simple and small change, so I don't really understand why it's waiting for 2 years at this point
It's pending further review because it's a very low priority, and a niche use case. We don't know if any changes are necessary, because we haven't had time to look.
There's over 300 other PRs open at the time of writing, many of which are "simple changes" that have been waiting far longer than this PR to come up in review priority.
If this is critical to your workflow, we recommend pulling the change in to a fork and building yourself.
Thanks for your understanding.
