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

Markers can have end time #5311

Merged
merged 10 commits into from
Nov 2, 2024
Merged

Conversation

MinasukiHikimuna
Copy link
Collaborator

Other metadata sources such as ThePornDB and timestamp.trade support end times for markers but Stash did not yet support saving those. This is a first step which only allows end time to be set either via API or via UI. Other aspects of Stash such as video player timeline are not yet updated to take end time into account.

  • User can set end time when creating or editing markers in the UI or in the API.
  • End time cannot be before start time. This is validated in the backend and for better UX also in the frontend.
  • End time is shown in scene details view or markers wall view if present.
  • Existing markers in the database will be updated to have -1 for end.
  • GraphQL API does not require end_seconds. Omitted end_seconds will default to -1.

Closes #3147.

@MinasukiHikimuna
Copy link
Collaborator Author

Added GraphQL documentation for seconds and end_seconds as recommended by stg-annon.

@MinasukiHikimuna
Copy link
Collaborator Author

There were two linting errors that weren't for some reason detected when I run make validate or even more specifically yarn run lint && yarn run check && yarn run format-check in the ui/v2.5. The errors should be fixed now but the Github pipeline needs to be run again.

@MinasukiHikimuna
Copy link
Collaborator Author

Forgot to run Prettier format after the lint fixes.

@BonerFide
Copy link

Not strictly related, but ....

Is it possible to have these also be sub-second? Since they're already a float maybe for end and start we can just skip the Math.round(getPlayerPosition() ?

I often find a part of a scene etc starts/ends at a weird spot, or a marker will need that precision to avoid starting at an awkward point etc.

@MinasukiHikimuna
Copy link
Collaborator Author

Not strictly related, but ....

Is it possible to have these also be sub-second? Since they're already a float maybe for end and start we can just skip the Math.round(getPlayerPosition() ?

I often find a part of a scene etc starts/ends at a weird spot, or a marker will need that precision to avoid starting at an awkward point etc.

I agree that it would be useful but as you said, it is not strictly related. This is just a first step in improving markers so I would prefer to get this merged rather than including all kinds of related changes. For example the player could surely be changed somehow with the introduction of end times but that should be in a separate PR.

@MinasukiHikimuna
Copy link
Collaborator Author

MinasukiHikimuna commented Oct 15, 2024

Updated the pull request on top of current develop so the migration numbers match.

Other metadata sources such as ThePornDB and timestamp.trade support end times for markers but Stash did not yet support saving those. This is a first step which only allows end time to be set either via API or via UI. Other aspects of Stash such as video player timeline are not yet updated to take end time into account.

- User can set end time when creating or editing markers in the UI or in the API.
- End time cannot be before start time. This is validated in the backend and for better UX also in the frontend.
- End time is shown in scene details view or markers wall view if present.
- Existing markers in the database will be updated to have -1 for end.
- GraphQL API does not require end_seconds. Omitted end_seconds will default to -1.
- 'seconds' is the required start time of a marker (in seconds).
- 'end_seconds' is an optional end time of a marker (in seconds).

Both fields support decimals.
@WithoutPants WithoutPants added the feature Pull requests that add a new feature label Nov 1, 2024
@WithoutPants WithoutPants added this to the Version 0.28.0 milestone Nov 1, 2024
@WithoutPants
Copy link
Collaborator

I've changed to use null values instead of -1. It's not great practice to use magic numbers like these to specify as null values if we can help it. I also changed the end label to End Time rather than Time End as it parses a bit better.

Otherwise, this is looking good. Thanks!

@WithoutPants WithoutPants merged commit 0d40056 into stashapp:develop Nov 2, 2024
2 checks passed
@SyZeeGee
Copy link

SyZeeGee commented Nov 3, 2024

Very neat feature!

Any chance that the scrubber can be easily updated to show the end time as well? I can open as a new issue as well, just wanted to ask here first (in case discussed elsewhere)

Quick mock (shows both scrubber and preview start / end range - colors can be different and are not too important as long as not jarring)
Marker_Start_End

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull requests that add a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Optional end for markers to use them as clips
4 participants