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

chore(web): transcoding settings cleanup #14765

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

pyorot
Copy link
Contributor

@pyorot pyorot commented Dec 18, 2024

hello hello and merry christmas 🎄 wanted to leave you a little change i've had in mind for you to look at in the new year

basically, sorting the transcoding settings in the web admin panel into a more logical order, partitioning them into two new accordions (for a total of four), and adding the localisation strings for these new panels. once i can post some screenshots of the new layout here, i'll mark this ready for review

EDIT: i tried building this on my windows 10 laptop to test the changes and... well... it installed over a gigabyte of crap getting thru node, then npm, then typescript, only to fail the sdk build because windows installed a version of ubuntu from 2 years ago that itself could only install a version of node from 2 years before then and it was 2 versions below the prerequisite version. so i installed that with the node apt repo, at which point wsl used up so much memory and overheated the pc into a bsod and i started wondering whether a trivial layout change in immich web was worth destroying my laptop haha

so i'll try to just mimic what it should look like in my next post below, but i am fairly confident it will build correctly since it was just copy+pasting simple tag markup. we have two new accordions and rearrangements within them. notice that there are two transcode policy strings, one title-capitalised and one sentence-capitalised. they are called transcoding_policy and transcoding_transcode_policy depending on their nestedness, the obvious way round

@pyorot
Copy link
Contributor Author

pyorot commented Dec 18, 2024

Video Transcoding Settings

Manage which videos to transcode and how to encode them

Transcode Policy
Set when a video will be transcoded

  • Transcode policy
  • Accepted video codecs
  • Accepted audio codecs
  • Accepted containers

Encoding Options
Set codecs, resolution, quality and other options for the encoded videos

  • Video codec [capitalisation corrected]
  • Audio codec
  • Target resolution
  • Constant rate factor (-crf) *
  • Preset (-preset)
  • Maximum bitrate
  • Threads
  • Tone-mapping
  • Two-pass encoding

Hardware Acceleration
Experimental; much faster, but will have lower quality at the same bitrate
[contents unchanged]

Advanced
Encoding options most users should not need to change
[contents unchanged]

@pyorot pyorot marked this pull request as ready for review December 18, 2024 04:26
Copy link
Contributor

@mertalev mertalev left a comment

Choose a reason for hiding this comment

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

I didn't really look at the changes to the setting components yet, just the text.

i18n/en.json Outdated
@@ -279,7 +279,7 @@
"transcoding_accepted_containers_description": "Select which container formats do not need to be remuxed to MP4. Only used for certain transcode policies.",
"transcoding_accepted_video_codecs": "Accepted video codecs",
"transcoding_accepted_video_codecs_description": "Select which video codecs do not need to be transcoded. Only used for certain transcode policies.",
"transcoding_advanced_options_description": "Options most users should not need to change",
"transcoding_advanced_options_description": "Encoding options most users should not need to change",
Copy link
Contributor

Choose a reason for hiding this comment

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

There's nothing inherently specific to encoding about this section; it just happens to not have other settings at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair; I'll revert this one. I'll drop my rationale for the change below:

I changed it to signpost that it's currently a continuation of encoding options, since I was worried about having some of those options outside the Encoding Options category. At first, I considered nesting Advanced under Encoding Options, but felt the flat structure is nicer aesthetically and less likely for settings to get lost. I also felt it unlikely there would ever be an advanced option not to do with encoding.

Copy link
Contributor

Choose a reason for hiding this comment

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

There was actually a tone-mapping option here until 1.120.0. There could be other niche options in the future as well, like setting frame rate or adding a custom flag to FFmpeg for debugging purposes.

Copy link
Contributor Author

@pyorot pyorot Dec 18, 2024

Choose a reason for hiding this comment

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

Yeah that last one isn't really to do with encoding. Currently, I have (or would have) the other two under encoding tho (were they not advanced); that's for later review.

i18n/en.json Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants