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

output-timer: Fixed buttons so that stopping a timer does not stop stream/recording #10055

Closed

Conversation

marktheminer
Copy link

Description

I made it so that stopping a stream/recording timer does not cause the stream/recording to stop.

This was done by adding two member variables to the OutputTimer class and by then referencing them appropriately at the correct places.

Motivation and Context

Multiple users including myself have had this happen while streaming when we set our timer and decide either that we want to stream longer or just set the timer incorrectly and need to change it.

https://obsproject.com/forum/threads/how-to-cancel-or-change-the-output-timer-once-started.137102/

#10026

How Has This Been Tested?

I compiled and ran it and did the following tests:

  1. Start the stream timer, make sure it starts the stream as before
  2. Stop the stream timer, make sure it doesn't stop the stream and still stops the timer
  3. Restart the timer and make sure it actually stops the stream when the time runs out

I did the same for the recording timer.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue UI/UX Anything to do with changes or additions to UI/UX elements. labels Jan 6, 2024
@therentabrain
Copy link

Thank you, thank you, thank you.
I don't see a way to upvote this for attention, so I comment with my thanks.

@marktheminer
Copy link
Author

You are welcome!

I see any interruption of stream as a major issue. At minimum, if this is not the right fix, it should prompt, "This will stop your stream, do you want to continue?"

…op stream/record

I made it so that stopping a stream/recording timer does not cause the
stream/recording to stop.  This was done by adding two member variables
to the OutputTimer class and by then referencing them appropriately at
the correct places.
@marktheminer marktheminer changed the title Fixed buttons on output timer so that stopping a timer does not stop stream/recording output-timer: Fixed buttons so that stopping a timer does not stop stream/recording Jan 12, 2024
Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

UI / UX review: the description of the new behavior makes sense. This needs testing to validate the behaviour. The Output Timer dialog needs serious reconsideration.

Please change the button labels to "Start timer" and "Stop timer"

@marktheminer
Copy link
Author

marktheminer commented Jan 18, 2024

@GeorgesStavracas after this merges, would you be interested in working together to come up with a design that re-worked the issues you see with the dialog as well as adding this?

#10103

I'm wondering if with a redesign if that could allow me to have that feature added, but in a different UI.

Maybe one of these three:

  1. a tab for streaming and one for recording (and then both recording timers could go on the recording group tab)
  2. two https://doc.qt.io/qt-6/qgroupbox.html, one for stream and one for recording (and then both recording timers could go on the recording group box)
  3. a Stream Timer dialog and a Recording Timer dialog

The difference in the code to do any of these 3 would be minimal.

Maybe I could mock them up if you don't have something like Figma to do it and we could iterate or something similar?

@GeorgesStavracas
Copy link
Member

The various OBS contributors agreed that the priority of this dialog is low, and some even raised the idea of removing it and making it an external tool. So I'm afraid the time and effort I'm willing to put on it is severely limited. Sorry.

@kjetilpp
Copy link

kjetilpp commented Apr 6, 2024

When will this be merged? :)

@marktheminer
Copy link
Author

@kjetilpp I don't think they are going to, but you can get it at https://obsproject.com/forum/resources/advanced-output-timer.1900/

image

@Fenrirthviti
Copy link
Member

Since this is currently available as a third party plugin, and we're not particularly keen on further changes to this dialog (and would prefer a refactor/rewrite instead), I will close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue UI/UX Anything to do with changes or additions to UI/UX elements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants