Skip to content

[Feature] Add Frames to VideoFile. UI and Graphql#6671

Open
bob12224 wants to merge 7 commits intostashapp:developfrom
bob12224:frames
Open

[Feature] Add Frames to VideoFile. UI and Graphql#6671
bob12224 wants to merge 7 commits intostashapp:developfrom
bob12224:frames

Conversation

@bob12224
Copy link
Contributor


name: Feature Addition
about: Add a feature to this project!
title: "[Feature] Add Frames to VideoFile. UI and Graphql"
labels: enhancement
assignees: 'WithoutPants, bnkai, Leopere'


Scope

Grabbing the Frame Count from the FFProbe (which is already done on all Video Files) and saving it into DB.
Then making the value available via GraphQl and in the Scene page, under File Info.

Closes/Fixes Issues

Other testing QA Notes

I have a full local dev server running. I have tested the migrations and manually adding files + scan. Frames is correctly calculated and saved into the DB. It is available in graphql, and I also added it to the File Info Screen (for Scene).

NOTE: I chose to not add this as a filter or sorting option. My needs are to read the value, so if anyone wants to filter/sort on Frames, I can add it.

#### Ubuntu

1. Install dependencies: `sudo apt-get install golang git gcc nodejs ffmpeg -y`
* To install `pnpm`: `npm install -g pnpm`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove this. Added it because the Pre-requisites kinda hides this for a new dev

"-show_format",
"-show_streams",
"-show_error",
"-count_frames",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There might be bigger changes required by this change. Now that we get the FrameCount by default, might be able to delete GetReadFrameCount function.

I leave larger refactoring to the maintainer to suggest

@feederbox826
Copy link
Collaborator

Your PR contains commits by a different user. Please verify that the commits are properly attributed to the authors you want

@Gykes
Copy link
Collaborator

Gykes commented Mar 17, 2026

Deferred till next release. Will review once this release closes out.

@Gykes Gykes self-assigned this Mar 17, 2026
@bob12224 bob12224 force-pushed the frames branch 2 times, most recently from 1e248a7 to 00d808c Compare March 17, 2026 03:15
@bob12224
Copy link
Contributor Author

bob12224 commented Mar 17, 2026

Your PR contains commits by a different user. Please verify that the commits are properly attributed to the authors you want

I have rebased and renamed the author of those commits. Thanks for the catch

@Gykes Gykes added the deferred Good feature that can be looked at for a later release. label Mar 17, 2026
Copy link
Collaborator

@feederbox826 feederbox826 left a comment

Choose a reason for hiding this comment

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

Not sure how useful this is to a broader audience, the ffprobe on post-migration is quite worrying. Frames can be esimated with duration * framerate and unless your entire library is restricted in duration, filters and sorting give you ????

Width int `json:"width"`
Height int `json:"height"`
Framerate string `json:"framerate"`
Frames int `json:"frame"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

inconsistent name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

VideoCodec: getFileStringValue(i, "videoCodec"),
AudioCodec: getFileStringValue(i, "audioCodec"),
FrameRate: getFileDuration(i) * 2,
Frames: int64(getFileDuration(i)) * 3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be duration * frameRate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

path := filepath.Join(dir, basename)

frames, err := m.ffprobe.GetReadFrameCount(path)
if err != nil || frames <= 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this the equality sign you want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IF (err != nil) || (frames <= 0) -> do not update row
IF (there was an error) OR (frames is not a valid number) -> do not update row

if frames is less than or equal to 0 that is considered an invalid number.
I could change it to <, but 0 frame video file seems invalid...

@@ -0,0 +1 @@
ALTER TABLE video_files ADD COLUMN frames INTEGER NOT NULL DEFAULT 0; No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not DEFAULT NULL and check for NULL in your postmigration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to use NULL here and in the postmigrate

@bob12224
Copy link
Contributor Author

Not sure how useful this is to a broader audience, the ffprobe on post-migration is quite worrying. Frames can be estimated with duration * framerate and unless your entire library is restricted in duration, filters and sorting give you ????

@feederbox826 While frames can be estimated, it is an estimate that can be incorrect (hence the PR). Frames might not be useful to the average user, but it it is critical for anyone using video editors that require frame accuracy (plugin developers). I am using OTIO and it requires accurate frames.

An alternative was described in the original ticket, but it seemed much more ambitious

Count int `db:"count"`
}{0}

if err := m.db.Get(&result, "SELECT COUNT(*) AS count FROM `video_files` WHERE `frames` IS NULL"); err != nil {
Copy link
Contributor Author

@bob12224 bob12224 Mar 18, 2026

Choose a reason for hiding this comment

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

@feederbox826 I could delete this 86_postmigrate.go and if people wanted to back fill their frames data, they could re-generate it from the tasks menu?

@feederbox826
Copy link
Collaborator

An alternative was described in the original ticket, but it seemed much more ambitious

since then, custom filed support has landed for scenes specifically #6584 #6601

would this not address your usecase?

@bob12224
Copy link
Contributor Author

An alternative was described in the original ticket, but it seemed much more ambitious

since then, custom filed support has landed for scenes specifically #6584 #6601

would this not address your usecase?

custom fields would address my issue.

I hesitate to close this PR though, if anyone else wants to frames per video, they would need to download a plugin and frames would not be available in the file info tab

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deferred Good feature that can be looked at for a later release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Frames to all video files

3 participants