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

feat(server, web): Added TranscodePolicy "Bitrate higher than max bitrate or not in accepted format" #6479

Merged
merged 12 commits into from
Jan 31, 2024

Conversation

Hely0n
Copy link
Contributor

@Hely0n Hely0n commented Jan 19, 2024

And here comes my last promise from #6319 :
The Bitratete policy.

This is just an additional transcode policy, which works like the "Transcode if higher than target resolution or not in desired format", but it will only transcode videos where the total bitrate is higher or equal than the bitrate set under "max bitrate".

image

This will allow admins to set the upload speed of the provider, where the server is running, as max bitrate so only videos that wouldn't be streamable due to their high bitrate will be transcoded regardless of their resolution.

This PR is build on top of my "Accepted Codecs"-PR ( #6460 ) .
With both features, the admin will have much more control about which videos have to be transcoded and thus allow for much better storage efficiency.

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.

There might be a bug in the bitrate check, but otherwise looks good (only looking at the bitrate changes since the other changes are in #6460)

server/src/infra/entities/system-config.entity.ts Outdated Show resolved Hide resolved
server/src/domain/media/media.service.ts Outdated Show resolved Hide resolved
@mertalev
Copy link
Contributor

@Hely0n I rebased the PR and it looks good. We can merge it once you add unit tests for the policy.

@Hely0n
Copy link
Contributor Author

Hely0n commented Jan 30, 2024

@Hely0n I rebased the PR and it looks good. We can merge it once you add unit tests for the policy.

Thank you very much. I plan to write them in the next days

@Hely0n
Copy link
Contributor Author

Hely0n commented Jan 31, 2024

@mertalev I hope this unit test meets your expectations. I just oriented on the resolution policy.

@mertalev mertalev merged commit 87c38d1 into immich-app:main Jan 31, 2024
23 checks passed
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.

None yet

4 participants