Skip to content

Conversation

@KelSolaar
Copy link
Member

References #336.

@KelSolaar KelSolaar requested a review from nick-shaw August 26, 2017 06:08
@coveralls
Copy link

coveralls commented Aug 26, 2017

Coverage Status

Coverage increased (+0.02%) to 98.197% when pulling 34cdc5b on feature/sony_slog into 124461c on develop.

Copy link
Contributor

@nick-shaw nick-shaw left a comment

Choose a reason for hiding this comment

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

For consistency, I think log_encoding_SLog3 and log_decoding_SLog3 should be updated to include the bit_depth, in/out_legal, and in/out_reflection kwargs that the other two have.

It won't work quite the same as the other two. For example, for log_encoding_SLog3, out_legal=True could just return the current y value. out_legal=False would need to return:
CV_to_IRE(y * (2**bit_depth - 1), bit_depth, True)/100

This is needed to make all three S-Logs able to return the values in the Table in the S-Log3 white paper, because the IRE column is based on out_legal=False, and the CV column is based on out_legal=True.

Make sense?

I would also argue for the default to be out_legal=True rather than the current False. This will produce float values compatible with the ACES IDTs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 98.155% when pulling 4e5f345 on feature/sony_slog into 124461c on develop.

@coveralls
Copy link

coveralls commented Sep 15, 2017

Coverage Status

Coverage increased (+0.02%) to 98.198% when pulling 4e5f345 on feature/sony_slog into 124461c on develop.

Copy link
Contributor

@nick-shaw nick-shaw left a comment

Choose a reason for hiding this comment

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

The function RGB_range in ycbcr.py has been replaced with the new CV_range function. However there are still references to RGB_range in the docstrings.

@coveralls
Copy link

coveralls commented Sep 16, 2017

Coverage Status

Coverage increased (+0.02%) to 98.198% when pulling 609166e on feature/sony_slog into 124461c on develop.

@coveralls
Copy link

coveralls commented Sep 16, 2017

Coverage Status

Coverage increased (+0.02%) to 98.198% when pulling 850df560d8d4b25eeedd77ee6e7ea266ab84d4ed on feature/sony_slog into 124461c on develop.

@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage increased (+0.02%) to 98.198% when pulling bccdbd84b7be8c5b537999e41288160561cf208c on feature/sony_slog into 124461c on develop.

…colour.log_encoding_SLog2" and "colour.log_decoding_SLog2" definitions use legal range.
@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage increased (+0.02%) to 98.198% when pulling aba669a on feature/sony_slog into 124461c on develop.

@KelSolaar
Copy link
Member Author

@nick-shaw : I have implemented the requested changes. The following snippet matches the table in [1]:

from colour import log_encoding_SLog2, log_encoding_SLog, log_encoding_SLog3

for x in [0.0, 0.18, 0.9]:
    print( x, "S-Log3 IRE", 100 * log_encoding_SLog3(x, bit_depth=10, out_legal=False))
    print( x, "S-Log2 IRE", 100 * log_encoding_SLog2(x, bit_depth=10, out_legal=False))
    print( x, "S-Log IRE", 100 * log_encoding_SLog(x, bit_depth=10, out_legal=False))
    print( x, "S-Log3 CV", 1023 * log_encoding_SLog3(x, bit_depth=10, out_legal=True))
    print( x, "S-Log2 CV", 1023 * log_encoding_SLog2(x, bit_depth=10, out_legal=True))
    print( x, "S-Log CV", 1023 * log_encoding_SLog(x, bit_depth=10, out_legal=True) )
0.0 S-Log3 IRE 3.53881278539
0.0 S-Log2 IRE 3.00012228519
0.0 S-Log IRE 3.00012228519
0.0 S-Log3 CV 95.0
0.0 S-Log2 CV 90.2810712183
0.0 S-Log CV 90.2810712183
0.18 S-Log3 IRE 40.6392694064
0.18 S-Log2 IRE 32.3449512215
0.18 S-Log IRE 37.6512722255
0.18 S-Log3 CV 420.0
0.18 S-Log2 CV 347.3417727
0.18 S-Log CV 393.825144695
0.9 S-Log3 IRE 60.9469471967
0.9 S-Log2 IRE 59.1365542288
0.9 S-Log IRE 65.3529251225
0.9 S-Log3 CV 597.895257443
0.9 S-Log2 CV 582.036215044
0.9 S-Log CV 636.491624073

References

  1. Sony Corporation. (n.d.). Technical Summary for S-Gamut3.Cine/S-Log3 and S-Gamut3/S-Log3. Retrieved from http://community.sony.com/sony/attachments/sony/large-sensor-camera-F5-F55/12359/2/TechnicalSummary_for_S-Gamut3Cine_S-Gamut3_S-Log3_V1_00.pdf

@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage decreased (-0.03%) to 98.156% when pulling 634348b055321f0d00f62ead9fcf1a5c0d4b36cc on feature/sony_slog into 124461c on develop.

@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage increased (+0.02%) to 98.2% when pulling 2af7b71 on feature/sony_slog into 124461c on develop.

@nick-shaw
Copy link
Contributor

That all looks good to me now.

@KelSolaar KelSolaar merged commit 72460f2 into develop Sep 18, 2017
@KelSolaar
Copy link
Member Author

Thanks @nick-shaw! :)

@KelSolaar KelSolaar deleted the feature/sony_slog branch September 18, 2017 23:36
@KelSolaar KelSolaar added this to the v0.3.11 milestone Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants