Clarify the difference between Pixel and Sample #92
Conversation
ffv1.md
Outdated
| @@ -23,17 +23,21 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S | |||
| ## Definitions | |||
|
|
|||
| -------- -------------------------------------------------------------- | |||
| `Sample`: The smallest addressable representation of a primary color in a frame. Examples of sample are Luminance, Blue Chrominance, Red Chrominance, Alpha, Red, Green, Blue. | |||
There was a problem hiding this comment.
I think the term primary color is misleading here, since a Y sample does not represent color. I see some language from RFC4175 which discusses this like:
A sample may represent a color component or a luminance component of the video.
ffv1.md
Outdated
| @@ -23,17 +23,21 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S | |||
| ## Definitions | |||
|
|
|||
| -------- -------------------------------------------------------------- | |||
| `Sample`: The smallest addressable representation of a primary color in a frame. Examples of sample are Luminance, Blue Chrominance, Red Chrominance, Alpha, Red, Green, Blue. | |||
|
|
|||
| `Pixel`: The smallest addressable representation of a color in a frame. It is composed of 1 or more samples. | |||
There was a problem hiding this comment.
Worthwhile to mention subsamples here when pixels share samples?
There was a problem hiding this comment.
IMO useful for clear split between pixel and sample.
But no strong opinion on keeping it.
There was a problem hiding this comment.
Nit: strictly speaking, luma ≠ luminance and chroma ≠ chrominance.
There was a problem hiding this comment.
You mean you prefer me to write luma and chroma?
There was a problem hiding this comment.
No, I mean that, if I’ll find the time, I would like to go through the full document and distinguish between.
Add: this might be a following PR.
ffv1.md
Outdated
| `ESC`: An ESCape symbol to indicate that the symbol to be stored is too large for normal storage and that an alternate storage method. | ||
|
|
||
| `MSB`: Most Significant Bit, the bit that can cause the largest change in magnitude of the symbol. | ||
|
|
||
| `RCT`: Reversible Color Transform, a near linear, exactly reversible integer transform that converts between RGB and YCbCr representations of a sample. | ||
| `RCT`: Reversible Color Transform, a near linear, exactly reversible integer transform that converts between RGB and YCbCr representations of a pixel. |
There was a problem hiding this comment.
Since Pixel is defined locally, I suggest to write it in Titlecase whenever it is used with the locally defined meaning.
| @@ -929,10 +933,10 @@ pseudo-code | type | |||
| Line( p, y ) { | | |||
| if (colorspace_type == 0) { | | |||
| for( x = 0; x < plane_pixel_width[ p ]; x++ ) | | |||
| Pixel( p, y, x ) | | |||
| Sample( p, y, x ) | | |||
There was a problem hiding this comment.
IIRC part of @dwbuiten was pointing out the need to define a Pixel/Sample function. Not blocking this commit or PR though.
JeromeMartinez
force-pushed
the
PixelSample
branch
from
28c3e0d
to
50017bc
Compare
|
PR update with |
| @@ -23,17 +23,21 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S | |||
| ## Definitions | |||
|
|
|||
| -------- -------------------------------------------------------------- | |||
| `Sample`: The smallest addressable representation of a color component or a luminance component in a frame. Examples of sample are Luminance, Blue Chrominance, Red Chrominance, Alpha, Red, Green, Blue. | |||
There was a problem hiding this comment.
Shouldn't this be luma and not luminance?
|
|
||
| `YCbCr`: A reference to the method of storing the value of a sample by using three numeric values that represent the luminance of the sample (Y) and the chrominance of the sample (Cb and Cr). | ||
| `YCbCr`: A reference to the method of storing the value of a Pixel by using three numeric values that represent the luminance of the Pixel (Y) and the chrominance of the Pixel (Cb and Cr). |
There was a problem hiding this comment.
(although this was already in the definition) should also be changed here?
There was a problem hiding this comment.
luma vs luminance. cc @retokromer who has corrected me on this in the past.
|
Here is an educated rant to support my requested change: http://poynton.ca/PDFs/YUV_and_luminance_harmful.pdf |
In this PR, I reused the wording near the change I do (e.g. for Sample definition I reused the content of YCbCr definition).
I suggest that it is not blocking for this PR and another (by @retokromer as he has more knowledge than me on this subject) PR would be done focused on this issue. If the PR about Luminance vs Luma is merged before this PR, I rebase and use the wording decided in the merged PR. Let's keep PR with only one subject, here the subject is Pixel vs Sample. |
|
👍 |
There are sometimes inversion between the meaning of each word. Trying to clarify, with definition.
Issue reported by @dwbuiten