[Feature] Add Frames to VideoFile. UI and Graphql#6671
[Feature] Add Frames to VideoFile. UI and Graphql#6671bob12224 wants to merge 7 commits intostashapp:developfrom
Frames to VideoFile. UI and Graphql#6671Conversation
| #### Ubuntu | ||
|
|
||
| 1. Install dependencies: `sudo apt-get install golang git gcc nodejs ffmpeg -y` | ||
| * To install `pnpm`: `npm install -g pnpm` |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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
|
Your PR contains commits by a different user. Please verify that the commits are properly attributed to the authors you want |
|
Deferred till next release. Will review once this release closes out. |
1e248a7 to
00d808c
Compare
I have rebased and renamed the author of those commits. Thanks for the catch |
feederbox826
left a comment
There was a problem hiding this comment.
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 ????
pkg/models/jsonschema/scene.go
Outdated
| Width int `json:"width"` | ||
| Height int `json:"height"` | ||
| Framerate string `json:"framerate"` | ||
| Frames int `json:"frame"` |
pkg/sqlite/setup_test.go
Outdated
| VideoCodec: getFileStringValue(i, "videoCodec"), | ||
| AudioCodec: getFileStringValue(i, "audioCodec"), | ||
| FrameRate: getFileDuration(i) * 2, | ||
| Frames: int64(getFileDuration(i)) * 3, |
There was a problem hiding this comment.
shouldn't this be duration * frameRate?
| path := filepath.Join(dir, basename) | ||
|
|
||
| frames, err := m.ffprobe.GetReadFrameCount(path) | ||
| if err != nil || frames <= 0 { |
There was a problem hiding this comment.
is this the equality sign you want?
There was a problem hiding this comment.
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 | |||
There was a problem hiding this comment.
why not DEFAULT NULL and check for NULL in your postmigration?
There was a problem hiding this comment.
updated to use NULL here and in the postmigrate
@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 { |
There was a problem hiding this comment.
@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?
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 |
name: Feature Addition
about: Add a feature to this project!
title: "[Feature] Add
Framesto 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 InfoScreen (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.