Re: DRM Atomic property for color-space conversion
From: Sharma, Shashank
Date: Thu Mar 16 2017 - 13:05:27 EST
Regards
Shashank
On 3/16/2017 5:55 PM, Brian Starkey wrote:
Hi,
On Thu, Mar 16, 2017 at 05:14:07PM +0200, Sharma Shashank wrote:
Regards
Shashank
On 3/16/2017 4:37 PM, Local user for Liviu Dudau wrote:
On Thu, Mar 16, 2017 at 04:30:59PM +0200, Ville Syrjälä wrote:
On Thu, Mar 16, 2017 at 04:20:29PM +0200, Sharma, Shashank wrote:
Regards
Shashank
On 3/16/2017 4:07 PM, Ville Syrjälä wrote:
On Tue, Jan 31, 2017 at 03:55:41PM +0000, Brian Starkey wrote:
On Tue, Jan 31, 2017 at 05:15:46PM +0200, Ville Syrjälä wrote:
On Tue, Jan 31, 2017 at 12:33:29PM +0000, Brian Starkey wrote:
Hi,
On Mon, Jan 30, 2017 at 03:35:13PM +0200, Ville Syrjälä wrote:
On Fri, Jan 27, 2017 at 05:23:24PM +0000, Brian Starkey wrote:
Hi,
We're looking to enable the per-plane color management
hardware in
Mali-DP with atomic properties, which has sparked some
conversation
around how to handle YCbCr formats.
As it stands today, it's assumed that a driver will
implicitly "do the
right thing" to display a YCbCr buffer.
YCbCr data often uses different gamma curves and signal
ranges (e.g.
BT.609, BT.701, BT.2020, studio range, full-range), so its
desirable
to be able to explicitly control the YCbCr to RGB conversion
process
from userspace.
We're proposing adding a "CSC" (color-space conversion)
property to
control this - primarily per-plane for framebuffer->pipeline
CSC, but
perhaps one per CRTC too for devices which have an RGB
pipeline and
want to output in YUV to the display:
Name: "CSC"
Type: ENUM | ATOMIC;
Enum values (representative):
"default":
Same behaviour as now. "Some kind" of YCbCr->RGB conversion
for YCbCr buffers, bypass for RGB buffers
"disable":
Explicitly disable all colorspace conversion (i.e. use an
identity matrix).
"YCbCr to RGB: BT.709":
Only valid for YCbCr formats. CSC in accordance with BT.709
using [16..235] for (8-bit) luma values, and [16..240] for
8-bit chroma values. For 10-bit formats, the range
limits are
multiplied by 4.
"YCbCr to RGB: BT.709 full-swing":
Only valid for YCbCr formats. CSC in accordance with
BT.709,
but using the full range of each channel.
"YCbCr to RGB: Use CTM":*
Only valid for YCbCr formats. Use the matrix applied via
the
plane's CTM property
"RGB to RGB: Use CTM":*
Only valid for RGB formats. Use the matrix applied via the
plane's CTM property
"Use CTM":*
Valid for any format. Use the matrix applied via the
plane's
CTM property
... any other values for BT.601, BT.2020, RGB to YCbCr etc.
etc. as
they are required.
Having some RGB2RGB and YCBCR2RGB things in the same property
seems
weird. I would just go with something very simple like:
YCBCR_TO_RGB_CSC:
* BT.601
* BT.709
* custom matrix
I think we've agreed in #dri-devel that this CSC property
can't/shouldn't be mapped on-to the existing (hardware
implementing
the) CTM property - even in the case of per-plane color
management -
because CSC needs to be done before DEGAMMA.
So, I'm in favour of going with what you suggested in the
first place:
A new YCBCR_TO_RGB_CSC property, enum type, with a list of fixed
conversions. I'd drop the custom matrix for now, as we'd need
to add
another property to attach the custom matrix blob too.
I still think we need a way to specify whether the source data
range
is broadcast/full-range, so perhaps the enum list should be
expanded
to all combinations of BT.601/BT.709 + broadcast/full-range.
Sounds reasonable. Not that much full range YCbCr stuff out there
perhaps. Well, apart from jpegs I suppose. But no harm in being
able
to deal with it.
(I'm not sure what the canonical naming for
broadcast/full-range is,
we call them narrow and wide)
We tend to call them full vs. limited range. That's how our
"Broadcast RGB" property is defined as well.
OK, using the same ones sounds sensible.
And trying to use the same thing for the crtc stuff is
probably not
going to end well. Like Daniel said we already have the
'Broadcast RGB' property muddying the waters there, and that
stuff
also ties in with what colorspace we signal to the sink via
infoframes/whatever the DP thing was called. So my gut
feeling is
that trying to use the same property everywhere will just end up
messy.
Yeah, agreed. If/when someone wants to add CSC on the output
of a CRTC
(after GAMMA), we can add a new property.
That makes me wonder about calling this one
SOURCE_YCBCR_TO_RGB_CSC to
be explicit that it describes the source data. Then we can
later add
SINK_RGB_TO_YCBCR_CSC, and it will be reasonably obvious that its
value describes the output data rather than the input data.
Source and sink have a slight connotation in my mind wrt. the
box that
produces the display signal and the box that eats the signal.
So trying
to use the same terms to describe the internals of the pipeline
inside
the "source box" migth lead to some confusion. But we do
probably need
some decent names for these to make the layout of the pipeline
clear.
Input/output are the other names that popped to my mind but
those aren't
necessarily any better. But in the end I think I could live
with whatever
names we happen to pick, as long as we document the pipeline
clearly.
Long ago I did wonder if we should just start indexing these
things
somehow, and then just looking at the index should tell you the
order
of the operations. But we already have the ctm/gamma w/o any
indexes so
that idea probably isn't so great anymore.
I want to avoid confusion caused by ending up with two
{CS}_TO_{CS}_CSC properties, where one is describing the data
to the
left of it, and the other describing the data to the right of
it, with
no real way of telling which way around it is.
Not really sure what you mean. It should always be
<left>_to_<right>_csc.
Agreed, left-to-right. But for instance on a CSC property
representing
a CRTC output CSC (just before hitting the connector), which
happens
to be converting RGB to YCbCr:
CRTC -> GAMMA -> RGB_TO_YCBCR_CSC
...the enum value "BT.601 Limited" means that the data on the
*right*
of RGB_TO_YCBCR_CSC is "BT.601 Limited"
On the other hand for a CSC on the input of a plane, which
happens to
be converting YCbCr to RGB:
RAM -> YCBCR_TO_RGB_CSC -> DEGAMMA
...the enum value "BT.601 Limited" means that the data on the
*left*
of YCBCR_TO_RGB_CSC is "BT.601 Limited".
Indicating in the property name whether its value is describing the
data on the left or the right is needed (and I don't think
inferring
that "it's always the YCBCR one" is the correct approach).
In my example above, "SOURCE_xxx" would mean the enum value is
describing the "source" data (i.e. the data on the left) and
"SINK_xxx" would mean the enum value is describing the "sink" data
(i.e. the data on the right). This doesn't necessarily need to
infer a
particular point in the pipeline.
Right, so I guess you want the values to be named "<a> to <b>" as
well?
Yes, I think we'll be wanting that as well.
So what we might need is something like:
enum YCBCR_TO_RGB_CSC
* YCbCr BT.601 limited to RGB BT.709 full
* YCbCr BT.709 limited to RGB BT.709 full <this would be the
likely default value IMO>
* YCbCr BT.601 limited to RGB BT.2020 full
* YCbCr BT.709 limited to RGB BT.2020 full
* YCbCr BT.2020 limited to RGB BT.2020 full
And thanks to BT.2020 we'll need a RGB->RGB CSC property as well.
Eg:
enum RGB_TO_RGB_CSC
* bypass (or separate 709->709, 2020->2020?) <this would be the
default>
* RGB BT.709 full to RGB BT.2020 full
I like this approach, from a point of view of being explicit and
discoverable by userspace. It also happens to map quite nicely to our
hardware... we have a matrix before degamma, so we could do a
CSC + Gamut conversion there in one go, which is apparently not 100%
mathematically correct, but in general is good enough.
... however having talked this over a bit with someone who understands
the detail a lot better than me, it sounds like the "correct" thing to
do as per the spec is:
CSC -> DEGAMMA -> GAMUT
e.g.
YCbCr bt.601 limited to RGB bt.601 full -> degamma ->
RGB bt.601 full to RGB bt.709 full
So that sounds like what we need to support in the API, and also
sounds more like the "separate properties" approach.
I agree.
Alternatives would involve two properties to define the input and
output
from the CSC separately, but then you lose the capability to see
which
combinations are actually supoorted.
I was thinking about this too, or would it make more sense to
create two
properties:
- one for gamut mapping (cases like RGB709->RGB2020)
- other one for Color space conversion (cases lile YUV 709 -> RGB
709)
Gamut mapping can represent any of the fix function mapping,
wereas CSC
can bring up any programmable matrix
Internally these properties can use the same HW unit or even same
function.
Does it sound any good ?
It seems to me that actually the two approaches can be combined into
the same thing:
* We definitely need a YCbCr-to-RGB conversion before degamma
(for converting YUV data to RGB, in some flavour)
* We definitely need an RGB-to-RGB conversion after gamma to handle
709 layers blended with Rec.2020.
The exact conversion each of those properties represents (CSC + gamut,
CSC only, gamut only) can be implicit in the enum name.
For hardware which has a fixed-function CSC before DEGAMMA with a
matrix after DEGAMMA, I'd expect to see something like below. None of
the YCBCR_TO_RGB_CSC values include a gamut conversion, because that
is instead exposed with the RGB_TO_RGB_CSC property (which represents
the hardware matrix)
YCBCR_TO_RGB_CSC (before DEGAMMA):
YCbCr BT.601 limited to RGB BT.601 full
YCbCr BT.709 limited to RGB BT.709 full
YCbCr BT.2020 limited to RGB BT.2020 full
RGB_TO_RGB_CSC (after DEGAMMA):
RGB BT.601 full to RGB BT.709 full
RGB BT.709 full to RGB BT.2020 full
On the other hand, on hardware which does a CSC + Gamut conversion in
one go, before DEGAMMA (like ours), you might have:
YCBCR_TO_RGB_CSC (before DEGAMMA):
YCbCr BT.601 limited to RGB BT.601 full
YCbCr BT.601 limited to RGB BT.709 full
YCbCr BT.709 limited to RGB BT.709 full
YCbCr BT.2020 limited to RGB BT.2020 full
RGB_TO_RGB_CSC (after DEGAMMA):
Not supported
Userspace can parse the two properties to figure out its options to
get from desired input -> desired output. It is perhaps a little
verbose, but it's descriptive and flexible.
Seems to be a good idea, Ville ?
- Shashank
It's certainly possible. One problem is that we can't inform userspace
upfront which combinations are supported. Whether that's a real
problem
I'm not sure. With atomic userspace can of course check upfront if
something can be done or not, but the main problem is then coming up
with a fallback strategy that doesn't suck too badly.
The approach above helps limit the set exposed to userspace to be only
those which are supported - because devices which don't have separate
hardware for the two stages won't expose values for both.
Anyways, I don't think I have any strong favorites here. Would be nice
to hear what everyone else thinks.
I confess to a lack of experience in the subject here, but what is
the more common
request coming from userspace: converting YUV <-> RGB but keeping
the gammut mapping
separate, or YUV (gammut x) <-> RGB (gammut y) ? In other words: I
can see the usefulness
of having an explicit way of decomposing the color mapping process
and control the
parameters, but how often do apps or compositors go through the
whole chain?
Right now, more or less the interest is on the RGB->YUV conversion
side, coz till now BT 2020 gamut was not in
picture. REC 601 and 709 have very close gamuts, so it was ok to
blend frames mostly without bothering about
gamut, but going fwd, ones REC 2020 comes into picture, we need to
bother about mapping gamuts too, else
blending Rec709 buffers and Rec2020 buffers together would cause very
visible gamut mismatch.
So considering futuristic developments, it might be ok to consider
both. Still, as Ville mentioned, it would be good
to hear from other too.
Yeah I agree that we definitely need to consider both for anything we
come up with now.
Cheers,
Brian
- Shashank
Best regards,
Liviu
--
Ville Syrjälä
Intel OTC