colour icon indicating copy to clipboard operation
colour copied to clipboard

[FEATURE]: Aliases for ffmpeg tone curves and primaries

Open fxthomas opened this issue 2 years ago • 10 comments

Ffmpeg provides a large set of primaries and transfer functions (OETFs) for use as metadata in video formats, with the -color_trc and -color_primaries parameters (or equivalent AVFrame members).

Allowed values for these options are stated to come out of ISO/IEC 23001-8:2013 in libavutil/pixfmt.h, and reference existing standards and recommendations that are for the most part already implemented in colour-science.

Is there any interest in adding aliases to let people reference these more easily?

fxthomas avatar May 13 '22 12:05 fxthomas

Hi @fxthomas,

I don't have a personal use case but it could be useful. We would need to source the standard though. Here are a few pages: https://cdn.standards.iteh.ai/samples/69661/48c09dfd01a840e2b7c99c70dc5338b1/ISO-IEC-23001-8-2016.pdf

KelSolaar avatar May 14 '22 06:05 KelSolaar

I'm willing to create PR for that, how should I organize this within the colour package? Would something like the following work?

The standard explicitly define values for these constants & enums:

  • colour.models.rgb.datasets.iso_iec_23001_8_2013.Iso23001ColourPrimaries: An integer enum containing the keys defined in §7.1
  • colour.models.rgb.datasets.iso_iec_23001_8_2013.PRIMARIES_ISO_IEC_23001_8_2013: a dict mapping Iso23001ColourPrimaries values to the actual primaries
  • colour.models.rgb.datasets.iso_iec_23001_8_2013.CCS_WHITEPOINTS_ISO_IEC_23001_8_2013: a dict mapping Iso23001ColourPrimaries values to the white point chromaticity coordinates
  • colour.models.rgb.transfer_functions.iso_iec_23001_8_2013.Iso23001TransferCharacteristics: An integer enum containing keys defined in §7.2
  • colour.models.rgb.transfer_functions.iso_iec_23001_8_2013.OETFS_ISO_IEC_23001_8_2013: a dict mapping Iso23001TransferCharacteristics values to the actual OETFs

The following can be derived from the standard and should IMO be also added for consistency with the other modules:

  • colour.models.rgb.datasets.iso_iec_23001_8_2013.WHITEPOINTS_NAMES_ISO_IEC_23001_8_2013: a dict mapping Iso23001ColourPrimaries values to the white point names
  • colour.models.rgb.datasets.iso_iec_23001_8_2013.MATRICES_ISO_IEC_23001_8_2013_TO_XYZ: a dict mapping Iso23001ColourPrimaries values to the associated RGB to XYZ matrix
  • colour.models.rgb.datasets.iso_iec_23001_8_2013.MATRICES_XYZ_TO_ISO_IEC_23001_8_2013: a dict mapping Iso23001ColourPrimaries values to the associated XYZ to RGB matrix

fxthomas avatar May 14 '22 14:05 fxthomas

@fxthomas : Overall this looks good to me with a few minor caveats:

  • We could shorten the module name to maybe iso_iec_23001_8
  • We tend/try to describe all our objects with what they are first and then the variant/method, e.g. Type_Method, even if it means not respecting Python style, thus Iso23001ColourPrimaries would probably be better named along those lines: ColourPrimaries_ISO23001_8, it becomes useful for auto-completion in the interpreter, e.g.
In [1]: import colour

In [2]: colour.models.eotf
 eotf()                  eotf_BT2100_PQ          eotf_inverse            eotf_inverse_BT2100_PQ  eotf_inverse_sRGB       eotf_sRGB              
 eotf_BT1886             eotf_DCDM               eotf_inverse_BT1886     eotf_inverse_DCDM       eotf_inverse_ST2084     eotf_ST2084            
 eotf_BT2100_HLG         eotf_DICOMGSDF          eotf_inverse_BT2100_HLG eotf_inverse_DICOMGSDF  eotf_SMPTE240M

Here the standard has also been superseeded by ISO/IEC 23091-2, thus we could "potentially" have ColourPrimaries_ISO23001_8 and ColourPrimaries_ISO23091_2. I don't think we need but it was to highlight the point! :)

We will source the above standard so that we can refer to it.

KelSolaar avatar May 14 '22 22:05 KelSolaar

Good points, thanks for the logical explanation!

Here the standard has also been superseeded by ISO/IEC 23091-2, thus we could "potentially" have ColourPrimaries_ISO23001_8 and ColourPrimaries_ISO23091_2. I don't think we need but it was to highlight the point! :)

So far I was going to use the original 2013 standard for all module/variable names and reference the new version in the module docstring, does something like this sound okay?

"""
ISO/IEC 23001-8 colour primaries
================================

Defines aliases to a set of standard colour primaries used in video encoding metadata, as defined by
ISO/IEC 23001-8.

These primaries are widely in use in video encoding and decoding software libraries, including ffmpeg.

Notes
------
-   :cite:`InternationalOrganizationforStandardization2013` has been superseded by
    :cite:`InternationalOrganizationforStandardization2021` but defines the same primaries.

References
----------

-   :cite:`InternationalOrganizationforStandardization2013` : International
    Organization for Standardization. (2013). INTERNATIONAL STANDARD ISO/IEC
    23001-8:2013 - Information technology - MPEG systems technologies - Part 8:
    Coding-independent code points, §7.1 "Colour primaries"
-   :cite:`InternationalOrganizationforStandardization2021` : International
    Organization for Standardization. (2021). INTERNATIONAL STANDARD ISO/IEC
    23091-2:2021 - Information technology - Coding-independent code points -
    Part 2: Video, §8.1 "Colour primaries"
"""

fxthomas avatar May 15 '22 20:05 fxthomas

I also noticed that ITU-T H.273 is available publicly, and was published after the original 2013 ISO/IEC standard with what appears to be the exact same content. I'll double-check that and add it to the references.

Given the number of different standards should we perhaps name the modules colour.models.rgb.datasets.video_metadata, colour.models.rgb.datasets.video_primaries, or even putting everything one level up in colour.models.rgb.video ; what do you think?

fxthomas avatar May 15 '22 21:05 fxthomas

If they are all the same we could alias them as an option, we sometimes do that, e.g. https://github.com/colour-science/colour/blob/develop/colour/characterisation/datasets/colour_checkers/sds.py#L3188, then we would describe that they are all the same in the doctrings. colour.models.rgb.video works for me!

KelSolaar avatar May 16 '22 11:05 KelSolaar

H.273 is the main reference. Please do not use ISO copy paste.

options are stated to come out of ISO/IEC 23001-8:2013 in

In fact there is no even 2013 string in the header file, I removed it here https://github.com/FFmpeg/FFmpeg/commit/6b1268f8c371c2e6efa5c8465b85ae9025254184.

useful. We would need to source the standard though

This standard is also duplicated in avc ISO spec that is not controlled. 14496-10:2020 here https://standards.iso.org/ittf/PubliclyAvailableStandards/index.html

ValZapod avatar May 20 '22 22:05 ValZapod

FWIW, I also prefer to reference H.273 as it is publicly available.

Kevin

KevinJW avatar Jun 09 '22 12:06 KevinJW

H.273 is the main reference. Please do not use ISO copy paste.

FWIW, I also prefer to reference H.273 as it is publicly available.

That's what I'm going to go for, I think, since it's the most easily accessible document. I'll just mark the extra standards as an extra reference.

In fact there is no even 2013 string in the header file, I removed it here https://github.com/FFmpeg/FFmpeg/commit/6b1268f8c371c2e6efa5c8465b85ae9025254184.

Nice, thanks, that makes it easier if the comment uses an up-to-date standard! (And the luma vs luminance changes is also one of my pet peeves of things easily confused... it's nice to get it correct :wink:)

fxthomas avatar Jun 10 '22 17:06 fxthomas

And the luma vs luminance changes

Well, technically we do support linear transfer (so Y of YCbCr is indeed Y of XYZ) just not in ffplay directly (strange open bug), alas, but mpv that uses ffmpeg almost in full, does support it using ffmpeg primitives.

Check with mpv vs ffplay: https://forum.videohelp.com/attachments/43324-1507526652/dpx-sequence.zip

We do (at least with zscale) support constant luminance, where Y of YCbCr is indeed Y of XYZ since you first apply the matrix on to get linear Y of YCbCr and only then apply nonlinearity to get Y'.

ValZapod avatar Jun 10 '22 17:06 ValZapod