Re: [PATCH V8 43/43] drm/colorop: Add destroy functions for color pipeline

2025-04-10 Thread Simon Ser
On Tuesday, April 1st, 2025 at 04:42, Alex Hung  wrote:

> On 3/29/25 09:48, Simon Ser wrote:
> 
> > I would prefer these functions to be introduced together with the
> > patches adding functions to create objects and adding the new fields.
> > That way it's easier to check the symmetry and at no point in the
> > series there are memory leaks.
> 
> The object creation and new fields are introduced in different patches.
> I divided this patch by introducing these functions in a patch, and 2.
> adding callers when needed to avoid memory leaks.

Could we introduce the destructor together with the function creating a
colorop, as a thin kfree() wrapper, and add cleanup for new fields in
that destructor in the same commit where the new fields are introduced?

> > Additionally, I would avoid using the name "cleanup", which seems to
> > have different semantics: for instance drm_plane_cleanup() doesn't kfree
> > the pointer. "destroy" seems more appropriate here.
> 
> How about the following changes, i.e., freeing pointer is moved out of
> the cleanup function, and keeping the names.

I don't really see the upside, because (1) the creator function allocates
(so there is an asymmetry between the creator and destructor) and (2) all
callers of the destructor function will always want to kfree(), but I'd be
fine with that.


[RFC] protocol: Suggestion for wl_data_offer.finish request description improvement

2025-04-10 Thread hippyandy
Hi all,

I hope this email finds you well. I’ve been reviewing the documentation for the 
wl_data_offer.finish request and would like to suggest a few minor improvements 
to enhance clarity and correctness.







Current Description:

``

Notifies the compositor that the drag destination successfully finished the 
drag-and-drop operation.

Upon receiving this request, the compositor will emit 
wl_data_source.dnd_finished on the drag source client.

It is a client error to perform other requests than wl_data_offer.destroy after 
this one.

It is also an error to perform this request after a NULL mime type has been set 
in wl_data_offer.accept or no action was received through wl_data_offer.action.

If wl_data_offer.finish request is received for a non drag and drop operation, 
the invalid_finish protocol error is raised.



"It is also an error to perform this request after a NULL mime type has been 
set in wl_data_offer.accept or no action was received through 
wl_data_offer.action."

``







Suggested Changes:

Replace "no action" with "none action" in the sentence:

Original: "or no action was received through wl_data_offer.action."

Suggested: "or none action was received through wl_data_offer.action."

(Note: This change assumes "none action" is the intended phrasing. If "no 
action" is technically correct, please disregard this suggestion.)

Clarify the subject in the last sentence for better readability:

Original: "If wl_data_offer.finish request is received for a non drag and drop 
operation..."

Suggested: "If the compositor received wl_data_offer.finish request for a non 
drag and drop operation..."







Rationale:

The first change aligns with potential grammatical preferences (though "no 
action" may also be correct; please verify the intended phrasing).

The second change makes it explicit that the compositor is the entity handling 
the request, improving clarity.

These adjustments aim to make the documentation more precise while retaining 
its original meaning. Thank you for considering these suggestions. Please let 
me know if further clarification would be helpful.

Best regards,




苏怀安

Re: [PATCH V8 06/43] drm/colorop: Add 1D Curve subtype

2025-04-10 Thread Daniel Stone
Hi Harry,

On Tue, 8 Apr 2025 at 18:30, Harry Wentland  wrote:
> On 2025-04-08 12:40, Daniel Stone wrote:
> > OK, Harry's reply cleared that up perfectly - the flexibility that's
> > there at the moment is about being able to reuse colorops for CRTCs in
> > post-blend ops (great!), not shared between planes.
>
> Just to make sure we're talking about the same thing:
>
> The design intent is to allow drm_colorops on a crtc (once we implement
> that), not to be able to share the same drm_colorop between a plane and
> crtc.

Right, that's my understanding here.

Cheers,
Daniel


Re: [PATCH V8 06/43] drm/colorop: Add 1D Curve subtype

2025-04-10 Thread Simon Ser
On Tuesday, April 8th, 2025 at 18:40, Daniel Stone  wrote:

> > > 5. For a given colorop property, is it an invariant that the colorop
> > > will only appear in one color pipeline at a time? (I believe so, but
> > > this probably needs documenting and/or igt.)
> > 
> > I don't really understand why that would matter to user-space.
> 
> Plane A: COLOR_PIPELINE@123 = { 1D_CURVE@456 }
> Plane B: COLOR_PIPELINE@789 = { 1D_CURVE@456 }
> 
> If userspace wasn't defensive about this, it would program the curve
> for 456 twice, and unless they were the same you'd get undesirable
> results.
> 
> The existing implementation is fine here, I think it just needs better
> igt to codify the expectations we all have.

Oh right, the same pipeline can definitely not be used on two different
planes, because KMS properties are set on the colorop objects by
user-space to configure the pipeline.

> > > Either way, I suspect that clorop->plane is the wrong thing to do, and
> > > that it maybe wants to be a list of planes in the drm_colorop_state?
> > 
> > I don't think so, for a given plane, there can only be a single pipeline
> > active at a time.
> 
> Yeah, again that was just not having grasped that the colorop not
> being derived from the plane was actually about allowing for it to be
> attached to a single CRTC instead, rather than potentially multiple
> planes. I have no concerns around this.
> 
> As it stands, I've gone through the implementation pretty thoroughly,
> as well as our use of it in Weston. I'm happy with how it looks for
> pre-blend, and I'm even happier that the implementation is written to
> apply easily to apply to post-blend CRTC pipelines.

Sweet!


Re: [Cin] "Competitors" or do we want to stay alive?

2025-04-10 Thread Andrew Randrianasulu
пн, 7 апр. 2025 г., 17:18 Pekka Paalanen :

> On Mon, 7 Apr 2025 14:44:25 +0300
> Andrew Randrianasulu  wrote:
>
> > пн, 7 апр. 2025 г., 14:19 Pekka Paalanen :
> >
> > > On Fri, 4 Apr 2025 22:14:10 +0300
> > > Andrew Randrianasulu  wrote:
> > >
> > > > пт, 4 апр. 2025 г., 21:35 Andrea paz :
> > > >
>
> ...
>
> > > > > A theoretical question: can CinGG be adapted to work in Wayland or
> is
> > > > > it impossible? Has XWayland limitation?
> > >
> > > X11 and Wayland designs for color management are fundamentally
> > > incompatible.
> > >
> > > With X11, applications never tell the display server what kind of
> > > pixels they are producing or which, if any, color profile they used for
> > > the display. With Wayland, applications must describe their pixels to
> > > the display server. Since X11 applications tell nothing to the X
> server,
> > > Xwayland has no color information to forward to the Wayland compositor.
> > >
> > > There can be case-specific manual solutions, though, that rely on
> > > correctly configuring both the X11 apps and the Wayland compositor. X11
> > > apps need to be configured to render for a specific display profile,
> > > and the Wayland compositor needs to assume the X11 windows are rendered
> > > for the same specific display profile. How that is actually done is up
> > > for each X11 app and each Wayland compositor.
> > >
> >
> > Is there possibility for X extension for Xwayland allowing relatively
> > simple way to tell Xwayland what to do with each window/region?
>
> Hi,
>
> sure, but would it not be more useful to invest that effort in a proper
> Wayland integration?
>

Cinelerra-gg like other cinelerra forks uses custom gui library:

https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=tree;f=cinelerra-5.1/guicast;h=0c7946e891c758f1b4df89c8459cbac926bf4b3b;hb=HEAD

I have no idea how much effort will be needed to add Wayland to it.

We lost our main/only developer nearly 5 years ago, so all I can do is
fixing minor bugs and trying to catch up with ffmpeg API changes.

So, we are on look out for c++ developer who is not afraid of "legacy" and
will not try to 'refactor' it on first sight (efforts to refactor cinelerra
proved to be LONG at best and unworkable at worst).

I do not have HDR capable GPU/Monitor, and things most likely will remain
so in foreseeable future (I am officially invalid, so my income sources are
limited, and VERY limited by USA standarts. )

Even if cinelerra(-gg) might never rise up to prominent position in
Linux/*bsd world again - I still consider us useful as example of working
NLE with internal 32fp pipeline, hw decoding, some OpenGL integration.

So, if only more developers were not afraid to experiment with 'legacy
'. After all, retrocomputing is a thing now!




> X11 is fundamentally limited to max 32-bit pixels so its stuck at 30
> bpc max with only 2 bits of alpha. That's probably the only thing that
> cannot be worked around.
>


I am not sure we need alpha over video region on final output  right
now x11/x11 opengl outputs definitely live without. If linux DRM can
transmit 12bit hdmi/DP - may be this functionality can be implemented as
separate output module? I looked into how psychtoolbox implement HDR
rendering .. very funny, a bit like using Vulkan to put output in HDR mode
and doing framebuffer blit manually in shaders 


> > A bit like
> >
> >
> https://github.com/oyranos-cms/oyranos/blob/master/libxcm/include/X11/Xcm/Xcm.h
>
> That's an interesting one, I wasn't aware of this. I have to take some
> statements back.
>
> So this is communicating ICC profiles and associated window regions to
> an X11 compositing manager. If a profile covers the whole window, this
> should be relatively easy to hook up in a Wayland compositor's XWM
> (embedded X11 window manager).
>
> Being limited to ICC profiles is a hindrance for HDR, although the CICP
> extension for ICC does exist. Unfortunately CICP does not carry
> everything we have in the Wayland color-management protocol extension.
> Most notably, it misses the reference white level which would be
> necessary[1].
>


Somewhere there also was cinepaint fork (unrelated to cinelerra) that also
can be used for testing this functionality.


I suspect working with std body behind ICC will be ... challenging, if
anyone decided to add missing info into standart.


>
> Thanks,
> pq
>
> [1]:
> https://www.freelists.org/post/argyllcms/Standard-spec-for-ICC-file-that-can-be-used-for-HDR-calibration,1
>


Pipeline vs. no pipeline (Re: [PATCH V8 06/43] drm/colorop: Add 1D Curve subtype)

2025-04-10 Thread Pekka Paalanen
On Tue, 8 Apr 2025 13:30:46 -0400
Harry Wentland  wrote:

> On 2025-04-08 12:40, Daniel Stone wrote:
> > Hi there,
> > 
> > On Tue, 1 Apr 2025 at 20:53, Simon Ser  wrote:  
> >> On Tuesday, April 1st, 2025 at 17:14, Daniel Stone  
> >> wrote:  

...

> >>> 1. Is it guaranteed that, if any plane on a device supports the
> >>> COLOR_PIPELINE property, all planes will support COLOR_PIPELINE?
> >>> (Given the amdgpu cursor-plane discussion, it looks like no, which is
> >>> unfortunate but oh well.)  
> >>
> >> I don't think so. (They could all expose a COLOR_PIPELINE with the only
> >> choice as the zero bypass pipeline, but that sounds silly.)  
> > 
> > Works for me: so any planes could not have colour pipelines, and the
> > result would be undefined (well, less defined) colour.  
> 
> Yes, basically it would be what we have now (without color pipelines).

Hi,

I see Alex wrote:

> In order to support YUV we'll need to add COLOR_ENCODING and COLOR_RANGE
> support to the color pipeline. I have sketched these out already but
> don't have it all hooked up yet. This should not hinder adoption of this
> API for gaming use-cases.

Was it considered to be able to lift the full-range RGB restriction
from the color pipelines, eventually leading to the possibility of
scanning out limited-range YCbCr bit-identical, giving userspace access
to the sub-black and super-white ranges for e.g. BT.814 purposes?

These questions are pointing in the direction of a bypass
COLOR_PIPELINE being different from no COLOR_PIPELINE. I assume a
bypass pipeline needs to shovel values through unchanged, while
"without color pipelines" would need the old COLOR_ENCODING and
COLOR_RANGE properties.

That reminds me of yet another question: if the framebuffer is limited
range, and it's not converted to full-range at the start of a color
pipeline, how will the sub-black and super-white ranges be represented?
Will they be negative and greater than 1.0 values, respectively? This
would be meaningful for the colorops being defined now, as I assume
people might implicitly limit their thinking to the [0.0, 1.0] range,
or at least exclude negative values.

The 3x4 CTM colorop is not yet explicit on whether it clamps its inputs
or outputs. Should all colorops be explicit about it?


Thanks,
pq


pgpfjkC7nKKip.pgp
Description: OpenPGP digital signature


Re: [Cin] "Competitors" or do we want to stay alive?

2025-04-10 Thread Pekka Paalanen
On Fri, 4 Apr 2025 22:14:10 +0300
Andrew Randrianasulu  wrote:

> пт, 4 апр. 2025 г., 21:35 Andrea paz :
> 
> > @Georgy
> > X11 supports CMS more or less well. Even I was able to profile the
> > monitor and install it with colord. It's not full support like Windows
> > or Apple has, but it's doable.

Hi,

X11 supports color management by having nothing to do with it. It's a
strictly hands-off design. Each application can choose a different CMS
simultaneously. An application that does not explicitly use a CMS also
does not have any color management with X11.

The X server can step out of the way and let applications post pixels
as-is to each monitor. This is good for application-side color
management, the pixel values do not get arbitrarily munged. However,
X11 also offers a number of interfaces to force the X server to munge
the pixel values. It allows loading a VCGT, but it also allows any
app to change any of the pixel munging settings, so the display
calibration is fragile.

On X11 it is the responsibility of each application to make each part
of their windows color-managed for the monitor they happen to overlap.
X11 does nothing to help with that.

> > For Wayland the big problem, why they didn't want to make a CMS, is
> > its being decentralized, a protocol to which various software could
> > interact by bringing their own version of CMS. The trouble is that a
> > CMS must be centralized by definition, if everyone fakes their own
> > implementation goodbye std.

In the Wayland model, the color management is centralized in the
Wayland compositor you use. Any application that does not choose to
manage its own colors will get color-managed by the compositor
automatically and unavoidably.

> > On Wayland, good programmers but bad color scientists, they didn't
> > even know how to start. Then Valve created The Gamescope to be able to
> > play games in HDR, and Wayland started from that to implement its CMS.

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/14
was created 5 years ago.

https://github.com/ValveSoftware/gamescope/pull/1168 was filed 1 year
ago, after the above had already settled on the fundamental information
to be carried.

EGL and Vulkan HDR extensions are the real API predecessors here for
both, and ITU-R recommendations and SMPTE specifications have had their
influence as well.

> > The result is not a real CMS but just something for video games and
> > movies. True, lately they seem to have figured out the problem and one
> > can hope that they will be able to solve it.

The decision to concentrate first to the HDR aspect was a deliberate
decision due to popular demand. The professional color-managed
workflows were always in my mind from the start. In fact, people were
in a hurry to implement HDR support without any thought to color
management, but I'm really happy to have been able to sell the idea
that color management is a prerequisite to HDR support. Otherwise we
would now have ad hoc HDR support that would quite likely be
fundamentally incompatible with the goals of color management.

> > For the time being, the main developer said:...
> > The color-management Wayland extension is enough for entertainment
> > purposes like games and movies. However, it is not enough for
> > professional color management needs including photo editing and print
> > preview. The major missing piece is the ability to measure the display
> > response. Every monitor is unique, and measuring is the only way to
> > achieve reliably repeatable and accurate display behavior.
> > ...
> >
> > Introduction to CMS in Wayland by the lead developer:
> >
> > https://www.collabora.com/news-and-blog/news-and-events/12-years-of-incubating-wayland-color-management.html

You quoted that part, but conveniently ignored everything else that
goes against your statements.

My plan for display profiling and calibration was documented 2 years
ago:
https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/27

and this was linked in the text you quoted, and below.

Note, that it is possible to calibrate and profile a monitor without
anything from issue 27, it's just tedious and error-prone to achieve.
Issue 27 makes that process easier to get right, more robust, and more
user friendly. The one thing that issue 27 does mention is the
programming of a VCGT, but I am unsure whether that has any benefit for
any HDR monitor driven in HDR mode.

> >
> > A summary by Paalanem of everything that happened in the 12 years of
> > development:
> > https://gitlab.freedesktop.org/pq/color-and-hdr
> >
> > Old discussion on monitor profiling (in the thread, user Graeme Gill
> > is ArgylCMS's developer):
> > https://gitlab.freedesktop.org/pq/color-and-hdr/-/issues/27
> >
> > Wayland CMS implementation in Gnome:
> > https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/4291
> >
> >
> > A theoretical question: can CinGG be adapted to work in Wayland or is
> > it impossible? Has XWayland limitation?

X11 and Wayland designs