Skip to content

Restructure ffmpeg#2392

Merged
WithoutPants merged 28 commits intostashapp:developfrom
WithoutPants:restructure-ffmpeg
Apr 18, 2022
Merged

Restructure ffmpeg#2392
WithoutPants merged 28 commits intostashapp:developfrom
WithoutPants:restructure-ffmpeg

Conversation

@WithoutPants
Copy link
Copy Markdown
Collaborator

Significant refactor of the ffmpeg package. Focus here was on making a more consistent and reusable package interface.

Changes include:

  • removed the stripExt parameter from NewVideoFile since this was used during scanning only
  • introduced fsutil.ReadLockManager to replace running stream checking code. Allows clients to "lock" a particular file for reading. When an operation requires to delete a file, it runs Cancel on all of the running lock contexts before returning to allow deletion.
  • eliminates ffprobe execution when stream transcoding files. Should improve performance.
  • moved frame rate calculation code into ffmpeg
  • moved most scene generation code into new scene/generate package
  • added ffmpeg/transcoder package to expose basic video and image manipulation functions: Transcode (re-encodes or copies a video with optional start time and duration), Splice (combines video files using a concat file), Screenshot (takes a screenshot at a given position), ImageThumbnail (makes a thumbnail of an image).
  • moved JSONTime into models/json package so that ffmpeg does not depend on models.
  • moved video phash generation code into new hash/videophash package. This should help formalise the algorithm.
  • added functions to ffmpeg for argument building

Most public interfaces are now documented according to godoc specs. The only thing that's still in a halfway state is the sprite generation stuff, which will need another refactor pass in future.

Biggest things to consider with this change are ensuring phash generation is consistent with the existing behaviour, ensuring all generation operations work correctly, ensuring live transcoding continues to work and that scene deletion follows existing behaviour with respect to running streams.

@WithoutPants WithoutPants added the chore Pull requests for refactoring and admin work label Mar 17, 2022
@WithoutPants WithoutPants added this to the Version 0.14.0 milestone Mar 17, 2022
@bnkai
Copy link
Copy Markdown
Collaborator

bnkai commented Apr 13, 2022

PR seems ok and tests ok.

Edit: Updated after b7cabfa
Checked

  • previews generation
  • screenshots generation
  • markers generation
  • phashes generation (phash values seem to remain the same as before for a few scenes i checked)
  • live transcoding seems to work ok
    Cancelation also seems to work if you try to delete a scene that is live transcoded

Todo ?

Comment thread internal/manager/task_transcode.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Pull requests for refactoring and admin work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants