Patman86 / x265-Mod-by-Patman

Patman's mod of x265
GNU General Public License v2.0
44 stars 3 forks source link

Use of binary prefix #1

Closed Dendraspis closed 3 years ago

Dendraspis commented 3 years ago

Hello @Patman86 ,

may I suggest using the correct binary prefix as shown here: https://en.wikipedia.org/wiki/Binary_prefix#Adoption_by_IEC,_NIST_and_ISO https://de.wikipedia.org/wiki/Bin%C3%A4rpr%C3%A4fix#IEC-Pr%C3%A4fixe_zur_Basis_2

As I saw Ki and Mi would be the appropriate prefixes instead of Kand M because the bytes are divided by 1024:

https://github.com/Patman86/x265-Mod-by-Patman/blob/7de4be112d7822b3bbfcb22715894a8b020f3b08/source/x265cli.cpp#L443-L472

On the other side using KB``andMB` would be better, when values would be divided by 1000. 😁

I'm glad you created these repositories. 👍 😄

Patman86 commented 3 years ago

I'll fix that.

But pls have a look here

Dendraspis commented 3 years ago

You mean that KB, MB, GB can be used for both? Yeah, this is probaby why Windows uses it and why it's so confusing sometimes. But meanwhile more and more apps and OSs switch to the IEC definition as far as I read.

It's not that important, but I had to make this suggestion after I checked if it's MB or MiB what I see. 😁

Patman86 commented 3 years ago

StaxRip itself uses

https://github.com/staxrip/staxrip/blob/462b19e0c320a2eb80488ebbbbdcff1f7014f8b1/Forms/MainForm.vb#L2840

Dendraspis commented 3 years ago

@Patman86 🤣 Guess who wrote this particular line a few months ago. 😁 👍 🤣

There is no problem using 1024 steps, but then the units should be KiB, MiB, ... If you want to use MiB thats absolutely okay, but I suggest using the binary prefix instead - so just adding some i to the code. 😁

But yes, it feels like I should change that and use \ 1000 and GB instead. 🤔

I hope we are talking (still) about the same thing. 🤔

Patman86 commented 3 years ago

I will adhere to the IEC and adopt the recommendations.

Dendraspis commented 3 years ago

Thanks. 😊

StaxRip itself uses https://github.com/staxrip/staxrip/blob/462b19e0c320a2eb80488ebbbbdcff1f7014f8b1/Forms/MainForm.vb#L2840

Just for the sake of completeness: 😉 https://github.com/staxrip/staxrip/commit/88acb9c5dcb22ead9c838265fa337f3722ea6d17

Patman86 commented 3 years ago

DONE Adjustments to match the SI prefix according to IEC @ ddd9c3f