Skip to content
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

Colorspace descriptions #31

Closed
wants to merge 1 commit into from

Conversation

dericed
Copy link
Contributor

@dericed dericed commented Dec 18, 2016

TO DO: remove redundancies with the ‘General Description’ section which
mostly focuses on colorspace

Comments welcome.

@dericed
Copy link
Contributor Author

dericed commented Dec 18, 2016

@@ -198,22 +198,51 @@ $$Q_{i}[a-b]=Table_{i}[(a-b)\&255]$$

## Colorspace

FFV1 supports two colorspaces: YCbCr and JPEG2000-RCT. In YCbCr, the Cb and Cr planes are optional, but MUST be used together if used. Omitting the Cb and Cr planes would code frames in grayscale without color data. For both YCbCr and JPEG2000-RCT, an optional Alpha plane can be used to code transparency data. In either colorspace, an FFV1 frame is composed of 1 to 4 planes that MUST use one of the following arragements:
Copy link
Contributor

Choose a reason for hiding this comment

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

the whole paragraph but the first sentence should be a bit below, after "### YCbCr"

@dericed dericed changed the title WIP Colorspace descriptions Colorspace descriptions Jan 7, 2017
@dericed
Copy link
Contributor Author

dericed commented Jan 7, 2017

I removed the redundancy between this new Colorspace section and the General Description (I may add to the General Description in a later PR). I also split lists of supported plane arrangements between YCbCr and JPEG2000-RCT as IIUC JPEG2000-RCT does not support Y or Y-alpha encoding though YCbCr does. Confirm?

Please re-review.

@michaelni
Copy link
Member

Theres a trailing whitespace, what is "arragements", did you run a spell checker on this ?
Cb and Cr were defined first as Y depends on their value but they do not depend on the Y value. That way nothing refers to a future definition
The text mixes U/V and Cb/Cr

@dericed
Copy link
Contributor Author

dericed commented Jan 7, 2017

  • I dropped the RCT reordering.
  • "arragements" -> "arrangements"
  • U/V corrected to Cb/Cr

@michaelni
Copy link
Member

The author name is dericed not Dave Rice

@dericed dericed force-pushed the colorspace-descriptions branch 3 times, most recently from b630414 to b585520 Compare January 7, 2017 23:54
@dericed
Copy link
Contributor Author

dericed commented Jan 7, 2017

authorship updated, ready for review again

- Y, Cb, Cr
- Y, Cb, Cr, Alpha

When FFV1 uses the YCbCr colorspace, the Y plane MUST be coded first. If the Cb and Cr planes are used then they MUST be coded after the Y plane. If an Alpha (transparency) plane is used, then it MUST be coded last.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to omit the (transparency), because for restoration purposes the Alpha channel can be used in other ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But is using alpha channel for non-transparency data a hack? If we remove transparency here, then we should also change the definition of alpha_plane as it references transparency.

Also other RFC's that reference alpha channels describe them as for transparency without acknowledgement of other purposes. See PNG: https://www.ietf.org/rfc/rfc2083.txt.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the way it has been defined and it’s actually used in (post-)production.

I was referring to film restoration, as we are looking at also from an archive’s point of view. Some scanners and restoration software use the alpha channel to match «problems» (e.g. scratches) and then to sweep them under the carpet – in order to avoid the user to see it, they increase a little the contrast, but that’s another topic. I don’t consider it a hack; it seems to me more like an alternate use.

I prefer without, but I’m happy with the majority’s point of view. What about something less absolute, like (e.g. transparency)? That would «allow» also other uses.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer without, but I’m happy with the majority’s point of view. What about something less absolute, like (e.g. transparency)? That would «allow» also other uses.

https://en.wikipedia.org/wiki/Alpha_compositing
"alpha compositing is the process of combining an image with a background to create the appearance of partial or full transparency"

From my point of view, alpha is tied to transparency, and other use is an hack (e.g. I already saw some images using RGBA channels and saying "warning, it is CMYK, but as there was no metadata for CMYK we used RGBA metadata in the stream", or YCbCr metadata is used when it is XYZ...); from my point of view if the 4th channel is used for something else e.g. scratches, it should have its own metadata saying that this is not transparency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me also alpha seems tied to transparency. The PNG spec seems to presume this. If somehow alpha does not mean transparency in the context of FFV1, then if someone converts RGBA PNG to RGBA FFV1, should the FFV1 have a different meaning? ie in PNG alpha means transparency, but in FFV1 should alpha's use be unclear?

I prefer to leave as transparency. Possibly there is a need for other channels, such as infrared scanning, but that is not analogous to alpha and should be in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK for keeping strictly transparency. I fully agree that the use made IRL is not a proper solution! An issue for this is open: #47

### JPEG2000-RCT

JPEG2000-RCT is a Reversible Color Transform that codes RGB (red, green, blue) planes losslessly in a modified YCbCr colorspace. Reversible conversions between YCbCr and RGB use the following formulae.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to mention bijective or bijective function somewhere, because this is what the formulae are.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://en.wikipedia.org/wiki/Bijection
"In mathematics, a bijection, bijective function or one-to-one correspondence is a function between the elements of two sets, where each element of one set is paired with exactly one element of the other set"

If I understand well, the formulae are not bijective because each element of YCbCr is not paired to something in RGB (there are some "illegal" values).
It is "reversible" as currently indicated in the spec (formulas permits to do RGB to YCbCr to RGB losslessly, but it does not permit to do YCbCr to RGB to YCbCr losslessly)

Copy link
Contributor

Choose a reason for hiding this comment

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

RGB -> YCbCr -> RGB losslessly, therefore RGB -> YCbCr -> RGB -> YCbCr -> RGB losslessly. Or do I miss something?

Copy link
Contributor

@JeromeMartinez JeromeMartinez Jan 8, 2017

Choose a reason for hiding this comment

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

RGB -> YCbCr -> RGB -> YCbCr -> RGB losslessly

Right, because the "illegal" values are not used in this case.
But YCbCr -> RGB -> YCbCr is not (because some values in YCbCr will not be converted losslessly).
"bijective" requests that both sets have the same count of elements "one-to-one correspondence" (this is not the case here, you have 2x more elements in YCbCr set than in RGB set)
In specs:
In the equation below, the term "bits" represents bits_per_raw_sample+1 for RCT or bits_per_raw_sample otherwise", see the "n+1" for RCT, the sets have not the same size so no bijection can exist.

This is, if I understand well the RCT, the reason this is reversible (you can revert the transformation from RGB) and not bijective (you can not revert from YCbCr if you start from it, with all values).

Copy link
Contributor

@retokromer retokromer Jan 8, 2017

Choose a reason for hiding this comment

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

I see, thank you! This must be the reason why many years ago we decided to work internally with big LUTs which are bijective (BTW working in Gray code instead of regular binary permits to reduce the rounding errors). Then I change my mind and do not suggest to mention bijective or bijective function.

Copy link
Member

Choose a reason for hiding this comment

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

The bijectivity of the RCT depends on the algebra it is used in. For example it is bijective in "Z,+,>>", it also is in "Z/nZ,+,>>" (the definition of >> does in fact not matter here as long as it is closed). In fact the RCT is bijective in a much wider range of cases, i think the set and + forming a quasigroup is sufficient. When the RCT is used as a mapping between sets of differing size it is of course not bijective.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. Thank very much you!

In JPEG2000-RCT colorspace, the coding order would be left to right and then top to bottom, with values interleaved by lines and stored in this order:

Y[1,1] Y[2,1] Cb[1,1] Cb[2,1] Cr[1,1] Cr[2,1] Y[1,2] Y[2,2] Cb[1,2] Cb[2,2] Cr[1,2] Cr[2,2]

## Coding of the sample difference

Instead of coding the n+1 bits of the sample difference with Huffman-, or Range coding (or n+2 bits, in the case of RCT), only the n (or n+1) least significant bits are used, since this is sufficient to recover the original sample. In the equation below, the term "bits" represents bits_per_raw_sample+1 for RCT or bits_per_raw_sample otherwise:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the dash after Huffman- needed in English? Huffman or Range coding looks better to me.

@dericed
Copy link
Contributor Author

dericed commented Jan 8, 2017

  • I updated Huffman- to Huffman as @retokromer suggests.
  • I did not reference the JPEG2000-RCT as bijective since it isn't.
  • I did not remove reference to alpha for transparency as suggested. Other opinions here?

@dericed
Copy link
Contributor Author

dericed commented Jan 8, 2017

OK, in this case, ready for re-review/merge.

@michaelni michaelni closed this in e69f975 Jan 8, 2017
@michaelni
Copy link
Member

This seems breaking xml build

ERROR: Unable to validate the XML document: draft-niedermayer-cellar-ffv1-01.xml
draft-niedermayer-cellar-ffv1-01.xml: Line 320: Element texttable content does not follow the DTD, expecting (preamble? , ttcol+ , c* , postamble?), got (c c c c)
draft-niedermayer-cellar-ffv1-01.xml: Line 321: Element t is not declared in c list of possible children
draft-niedermayer-cellar-ffv1-01.xml: Line 324: Element t is not declared in c list of possible children
draft-niedermayer-cellar-ffv1-01.xml: Line 328: Element t is not declared in c list of possible children
draft-niedermayer-cellar-ffv1-01.xml: Line 331: Element t is not declared in c list of possible children
make: *** [draft-niedermayer-cellar-ffv1-01.html] Error 1

@michaelni
Copy link
Member

fixed

@dericed
Copy link
Contributor Author

dericed commented Jan 8, 2017

Thanks, I just force pushed the same idea.

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