Fix the implementation of the HLG EOTF function #23
Conversation
| @@ -482,19 +482,24 @@ def eotf_hlg( | |||
| Returns: | |||
| frame_data: pixel values in the physical luminance up to 10,000 cd/m^2 | |||
| """ | |||
| frame_data = np.maximum(frame_data, 0.0) | |||
There was a problem hiding this comment.
According to the ITU-R Recommendation BT.2100, the values below 0 and above 1 are also possible and acceptable (see NOTE 5g).
There was a problem hiding this comment.
Yes, fine, note 5h per latest version.
799b2e0 to
8762be6
Compare
| if l_max <= 1000.0: | ||
| gamma = 1.2 | ||
| else: | ||
| gamma = 1.2 + 0.42 * np.log10(l_max/1000.0) |
There was a problem hiding this comment.
See NOTE 5e in the Recommendation BT.2100
There was a problem hiding this comment.
LGTM, it appears to be NOTE 5f though in the latest verson (https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.2100-2-201807-I!!PDF-E.pdf).
| # If frame_data was a color channel (R, G, or B), the correct implementation would be: | ||
| # (l_max - l_min) * np.power(Y, gamma - 1.0) * frame_data + l_min, | ||
| # where Y would be obtained from linearized R, G, B channels. | ||
| frame_data = (l_max - l_min) * np.power(frame_data, gamma) + l_min |
There was a problem hiding this comment.
If frame_data = Y the equation in the NOTE can be written as:
(l_max - l_min) * np.power(frame_data, gamma - 1.0 + 1.0) + l_min
There was a problem hiding this comment.
This part I don't see in BT.2100, but to be honest I have only looked very briefly.
There was a problem hiding this comment.
@slhck You are right, it is not in the recommendation as it only defines the functions for the individual components. The "most correct" way would therefore be to convert the YUV input to RGB, apply the EOTF on individual components, and convert back to YUV. But I believe that is not really needed for our use case and this solution seems like a reasonable approximation. Would you agree?
There was a problem hiding this comment.
I see, yes, that makes sense. I was more confused about the subtraction gamma - 1.0 in the original code and where that came from.
| if l_max <= 1000.0: | ||
| gamma = 1.2 | ||
| else: | ||
| gamma = 1.2 + 0.42 * np.log10(l_max/1000.0) |
There was a problem hiding this comment.
LGTM, it appears to be NOTE 5f though in the latest verson (https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.2100-2-201807-I!!PDF-E.pdf).
| @@ -482,19 +482,24 @@ def eotf_hlg( | |||
| Returns: | |||
| frame_data: pixel values in the physical luminance up to 10,000 cd/m^2 | |||
| """ | |||
| frame_data = np.maximum(frame_data, 0.0) | |||
There was a problem hiding this comment.
Yes, fine, note 5h per latest version.
|
Feel free to merge and fix the tests after, or change the values before! Thanks to @cosmin and you for providing the fix! |
|
Will do a release tomorrow, thanks! |
This PR fixes the implementation of HLG EOTF function. An important note is also added in case the function was to be used on channels other than Y.