-
Notifications
You must be signed in to change notification settings - Fork 53
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
Naming and settings fixes #304
Conversation
That seems right to me! |
One comment about the documentation: true lossless encoding no longer requires libx264rgb. I haven't tested that's it's completely lossless, but VideoIO now supports YUV444 encoding, not just YUV420, which at least avoids the chroma subsampling. |
I kept the renames as individual commits, so let me know if any should be dropped
This would be good to write up for the docs. Edit: oh right, that was your point.. would you mind writing that up? |
Sure! |
Codecov Report
@@ Coverage Diff @@
## master #304 +/- ##
=======================================
Coverage 82.35% 82.35%
=======================================
Files 17 17
Lines 1196 1196
=======================================
Hits 985 985
Misses 211 211
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a PR against this branch with my suggestions.
I just tested it and |
Even with |
Yeah. It goes through sws_scale for RGB->YUV which is notoriously complicated so it's perhaps not surprising that it's not lossless without spending the time to fine-tune the sws_scale parameters.
…On Mon, Mar 15, 2021, at 17:22, Ian Butterworth wrote:
Even with `color_range=2`? That's required for Gray, so might be needed for RGB
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUDZT3KTVCO56W2VDZ6NWLTDZ273ANCNFSM4ZHAHURA>.
|
@galenlynch if we rename
container_settings
->container_options
do we also want to:change
to
Note
OptionsT