Add support for higher precision pixel formats #1528

niklaspandersson · 2024-04-02T18:35:21Z

This branch introduces:

Required plumbing in accelerator / core / common / shell for managing frames with a higher than 8 bit precision pixel format
Basic support in ffmpeg producer for preserving precision of 10- and 12 bit content
Rudimentary support in the ffmpeg- and decklink consumers for consuming higher precision frames

Only minimal testing have been performed on windows.

Julusian

This looks like a good start, and doesn't architecturally conflict with what I had in mind.
It is rather minimal in what producers/consumers are supported, but I don't see that as a problem for now.

The only bit I don't like is the comment I added in array.h

src/common/array.h

src/modules/screen/consumer/screen_consumer.cpp

src/modules/decklink/consumer/frame.cpp

Julusian · 2024-04-03T15:05:11Z

src/modules/ffmpeg/producer/av_producer.cpp

@@ -502,7 +502,11 @@ struct Filter
                                              AV_PIX_FMT_ABGR,
                                              AV_PIX_FMT_YUV444P,
                                              AV_PIX_FMT_YUV422P,
+                                              AV_PIX_FMT_YUV422P10,


Perhaps this could include more formats?
In my testing, some codecs prefered to decode to RGB48/RGBA64, which should be trivial to handle.
And I was able to add the following yuv formats which I think only needed the handling on mapping into pixel_format:

AV_PIX_FMT_YUV444P10, AV_PIX_FMT_YUV422P10, AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV444P16, AV_PIX_FMT_YUV422P16, AV_PIX_FMT_YUV420P16, AV_PIX_FMT_YUVA444P10, AV_PIX_FMT_YUVA422P10, AV_PIX_FMT_YUVA420P10, AV_PIX_FMT_YUVA444P16, AV_PIX_FMT_YUVA422P16, AV_PIX_FMT_YUVA420P16,

This isn't necessary for merging this PR though.

Julusian · 2024-04-03T15:20:24Z

src/accelerator/ogl/util/device.cpp

        });
    }

 #ifdef WIN32
-    std::future<std::shared_ptr<texture>> copy_async(GLuint source, int width, int height, int stride)
+    /* Unused? */


yes, I forgot to remove this when removing the shared-texture flow for CEF. Feel free to delete it

src/accelerator/ogl/image/image_kernel.cpp

Julusian · 2024-04-03T15:37:26Z

src/modules/decklink/consumer/decklink_consumer.cpp

 {
    configuration config = parse_xml_config(ptree, format_repository);

+    config.hdr = (depth != common::bit_depth::bit8);
+
+    if (config.hdr && config.primary.has_subregion_geometry()) {


This is a shame, but I can appreciate that making all of these features behave correctly with 16bit will be hard and is not being asked for yet.
Unfortunately this means that it won't be possible to benefit from this work if a key output is needed, so this will only be usable in fullscreen channels, rather than keyed overlay channels. (This is not a problem, just writing out this limitation for when other users are curious)

niklaspandersson · 2024-04-05T15:34:59Z

I'm glad that you approve. This is just the first iteration, to get something out there that can be tested by the client. How "complete" the support will be will ultimately be up to them.

5opr4ni · 2024-04-08T11:19:09Z

Hi,

This is super interesting work and would have been very useful for Eurovision Song Contest 2024 in Malmö, where we're incorporating a lot of color gradients in the graphics. All the graphics for ESC will be done in CasparCG.

Keep up the good work, Niklas and Julian!

MauriceVeraart · 2024-04-09T09:05:50Z

Nice work. Can this also be used for "custom resolution consumers"?
10Bit or even higher will be a big improvement for our market with non-standard resolutions.

Julusian · 2024-04-17T15:48:06Z

src/modules/decklink/consumer/decklink_consumer.cpp

@@ -225,6 +225,8 @@ struct HDRMetadata
 const auto REC_709  = ChromaticityCoordinates{0.640, 0.330, 0.300, 0.600, 0.150, 0.060, 0.3127, 0.3290};
 const auto REC_2020 = ChromaticityCoordinates{0.708, 0.292, 0.170, 0.797, 0.131, 0.046, 0.3127, 0.3290};

+typedef const char* BSTR;


Theres already an typedef called String inside decklink_api.h which handles this abstraction.
So replace the uses of BSTR with String and it should be happy

Ah, that's great! Thank you

* Remove the 'magic numbers' related to YCbCr->RGB conversion for SD / HD * Replace with standard coefficients and matrices for bt.601 and bt.709 respectively

* Needed for providing necessary metadata to consumers about the frame's color_space * color_space is deduced from the color-depth channel config. A separate config could be added later * The fragment shader does not perform any color space conversions

src/accelerator/ogl/image/image_kernel.cpp

src/modules/decklink/producer/decklink_producer.cpp

src/modules/ffmpeg/producer/av_producer.cpp

Julusian · 2024-04-29T11:54:01Z

src/shell/server.cpp

@@ -255,15 +257,27 @@ struct server::impl

            auto format_desc_str = xml_channel.second.get(L"video-mode", L"PAL");
            auto format_desc     = video_format_repository_.find(format_desc_str);
+            auto color_depth     = xml_channel.second.get<unsigned char>(L"color-depth", 8);


I'm not following what this color_depth value is actually used for.
As far I can tell, it is being passed around a lot to then be only used by https://github.com/niklaspandersson/casparcg-server/blob/aac7d44da5e25613efd1a0ffcb53892a45062d8a/src/core/mixer/mixer.cpp#L77-L87, but does that need/want a color_space as at that point it is RGB?

It is used to determine whether to use one or two bytes per color channel in mixer-generated frames. It's needed to maintain the extra precision throughout the pipeline.

sorry, I meant to write this on the color-space parsing, not color-depth

Frames created by the mixer need to be tagged with an explicit color space for consumers such as decklink to be able to attach the correct metadata on their output.
It is also needed to perform correct color space transformations in the mixer when combining material from different color spaces (not implemented yet though)

It is also needed to perform correct color space transformations in the mixer when combining material from different color spaces (not implemented yet though)

I wouldn't expect that to be necessary, seeing how the output of the shader is rgb. So while frames may enter the mixer as different formats in varying color spaces, the output should always be rgb and surely agnostic of whatever the output color space will be?

Frames created by the mixer need to be tagged with an explicit color space for consumers such as decklink to be able to attach the correct metadata on their output.

I would expect this to be a property of the consumer, not the whole channel.

I haven't done much research into how HDR and its metadata works, but from what I have read I would expect that the only changes needed are to:

composite in 16bit rgb

use the correct algorithm for converting that to 10bit yuv using the chosen hdr mode and metadata values

so unless the mixer is going to be generating frames in yuv, does it need to know what the consumers are doing?

RGB is not color space agnostic. Max green, ie. vec3(0, 1.0, 0), represents different shades of green depending on which color space you're using.

Oh right. Having looked into it some more, you are right.
Sometimes referred to as wide and narrow gamut.

I'm still not entirely keen on this being done on the channel like this, but I dont have any better ideas right now. Especially as there isn't a convenient/cheap place to do a per-consumer color conversion

src/accelerator/ogl/util/device.cpp

src/modules/decklink/consumer/decklink_consumer.cpp

niklaspandersson · 2024-04-29T18:09:42Z

@MauriceVeraart

Nice work. Can this also be used for "custom resolution consumers"? 10Bit or even higher will be a big improvement for our market with non-standard resolutions.

Yes, absolutely. For ffmpeg I think it would just work without any modifications. Do have a specific consumer in mind?

MauriceVeraart · 2024-04-30T09:22:29Z

@MauriceVeraart

Nice work. Can this also be used for "custom resolution consumers"? 10Bit or even higher will be a big improvement for our market with non-standard resolutions.

Yes, absolutely. For ffmpeg I think it would just work without any modifications. Do have a specific consumer in mind?

When we use custom resolutions it will be the screenconsumer

…nk consumer

Julusian · 2024-05-22T08:55:41Z

@niklaspandersson I don't see any comments left that need addressing on this. Let me know if you are happy for it to be merged (or go ahead and merge it yourself)

niklaspandersson · 2024-05-23T13:23:25Z

@Julusian I'm happy if you are happy. Feel free to merge

Sidonai-1 · 2024-05-29T02:07:26Z

Great work, thank you!

Question: channel bit depth goes from 8 to 16 directly, right? I tried to set it to 10 or 12 but it crashed, I just wanted to quickly test it on a laptop without decklink to compare image outputs with Photoshop but it seems that is not possible at the moment.

Screen consumer only supports 8-bit color depth
Image consumer only supports 8-bit color depth.

Julusian · 2024-05-29T08:57:58Z

@Sidonai-1 correct, it is limited to 8 or 16bit
And yes, the consumer/producer support for higher bit depths is very limited currently

Julusian reviewed Apr 5, 2024

View reviewed changes

Julusian reviewed Apr 17, 2024

View reviewed changes

niklaspandersson force-pushed the 16bit branch 2 times, most recently from e323e81 to e8d8e88 Compare April 24, 2024 10:58

niklaspandersson added 4 commits April 29, 2024 06:53

Add support in ogl accelarator for higher color depths

12d27a0

Make consumer factories aware of color depth

3e1f1b1

Expose color depth in config

e4e0bcc

Add support for color depth in ffmpeg consumer

9a0f8e8

niklaspandersson force-pushed the 16bit branch from 10f39d6 to 2b3e2a4 Compare April 29, 2024 07:00

niklaspandersson added 11 commits April 29, 2024 08:26

Add support for higher color depths in ffmpeg producer

e3289ec

Add support for higher color depth in decklink consumer

e276ffb

Add support for higher color depth in decklink producer

b249493

Add uniforms for color matrix and luma coefficients

fe74379

* Remove the 'magic numbers' related to YCbCr->RGB conversion for SD / HD * Replace with standard coefficients and matrices for bt.601 and bt.709 respectively

Add color_space property to pixel_format_desc

a1c5e5f

Add support for bt.2020 in image kernel

1140077

Add correct color space metadata to frames from the decklink producer

0db73de

Add correct color space metadata to frames from the ffmpeg producer

8d5b7d7

Update decklink consumer HDR metadata

ebcc6ab

Add config parameter for channel color-space

aac7d44

niklaspandersson force-pushed the 16bit branch from 2b3e2a4 to aac7d44 Compare April 29, 2024 08:27

niklaspandersson marked this pull request as ready for review April 29, 2024 08:39

Julusian reviewed Apr 29, 2024

View reviewed changes

niklaspandersson added 2 commits May 6, 2024 09:12

[FIXUP] Clean up create_array signature

093bfcb

[FIXUP] Add comments related to paths supporting hdr frames in deckli…

1acf6bb

…nk consumer

niklaspandersson added 4 commits May 6, 2024 09:28

[FIXUP] Rename decklink producer param HDR to 10BIT

e5f8bd8

[FIXUP] Add bt601 as colorspace to decklink producer

bbda1bd

[FIXUP] Add bt601 as possible colorspace to ffmpeg producer

c50120d

[FIXUP] Move default decklink hdr colorspace to config

b69d46e

Merge branch 'master' into 16bit

9082117

Julusian merged commit 2ffc202 into CasparCG:master May 24, 2024
4 checks passed

Julusian added a commit that referenced this pull request May 24, 2024

fix: ensure frame has video before checking colorspace #1528

6a7c3c5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for higher precision pixel formats #1528

Add support for higher precision pixel formats #1528

niklaspandersson commented Apr 2, 2024

Julusian left a comment

Julusian Apr 3, 2024

Julusian Apr 3, 2024

Julusian Apr 3, 2024

niklaspandersson commented Apr 5, 2024

5opr4ni commented Apr 8, 2024

MauriceVeraart commented Apr 9, 2024

Julusian Apr 17, 2024 •

edited

niklaspandersson Apr 17, 2024

Julusian Apr 29, 2024

niklaspandersson Apr 29, 2024

Julusian Apr 29, 2024

niklaspandersson Apr 29, 2024

Julusian Apr 29, 2024

niklaspandersson Apr 30, 2024

Julusian Apr 30, 2024

niklaspandersson commented Apr 29, 2024

MauriceVeraart commented Apr 30, 2024

Julusian commented May 22, 2024

niklaspandersson commented May 23, 2024

Sidonai-1 commented May 29, 2024

Julusian commented May 29, 2024

Add support for higher precision pixel formats #1528

Add support for higher precision pixel formats #1528

Conversation

niklaspandersson commented Apr 2, 2024

Julusian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niklaspandersson commented Apr 5, 2024

5opr4ni commented Apr 8, 2024

MauriceVeraart commented Apr 9, 2024

Julusian Apr 17, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niklaspandersson commented Apr 29, 2024

MauriceVeraart commented Apr 30, 2024

Julusian commented May 22, 2024

niklaspandersson commented May 23, 2024

Sidonai-1 commented May 29, 2024

Julusian commented May 29, 2024

Julusian Apr 17, 2024 •

edited