WIP: Add EXT_config_drm_format_query extension

[amshafer]

This extension is a fairly trivial change to allow an application to get the DRM fourcc code for an EGLConfig. In some cases this is desirable, since you may only have an EGLSurface and its config but need to create other resources that match the surface. This can happen with EGLStreams apps or with EGL platform libraries.

From the overview:
This extension adds an EGLConfig attribute that specifies a DRM format, as defined in drm_fourcc.h. If an application needs to share buffers with other libraries or processes that require specific formats, then this allows an application to select a matching EGLConfig accordingly. EGLStream applications may need to know what DRM format a surface is using so that they can pass the format and any modifiers to other APIs or other stream consumers.

This extension allows passing EGL_LINUX_DRM_FOURCC_EXT to eglGetConfigAttrib, allowing the application to forward the format code as needed.

[stonesthrow]

This sounds very familiar, like it was proposed before.
So, EGLConfig has the EGL_NATIVE_VISUAL_ID to match up with code the Window system/ platform uses. And this extension could use that existing mechanism, and just say EGL_NATIVE_VISUAL_ID contains FourCC codes. BUT on some platforms I think this is already is already in use, and is not available for FourCC code.
The back of my brain is tingling that this whole conversation, proposal and comment, happened before.
I think it was related to Wayland?

[stonesthrow]

I think this is sounding familiar because of discussions on the proposal for https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_config_select_group.txt

[amshafer]

I know EGL_NATIVE_VISUAL_ID is another case of a query-only attribute for a config, but I do think there's value in keeping this separate. It makes more sense to explicitly ask for a DRM format vs asking for a visual ID that probably is a DRM format. I think it's a better idea for the "already in use" concerns that you outlined.

[cubanismo]

I reviewed this internally before Austin pushed it, as did @kbrenneman. I'm not aware of a prior proposal to expose this, at least not one that was merged. I agree with @amshafer's reasoning for keeping this separate from EGL_NATIVE_VISUAL_ID, as we already had that discussion internally as well.

[kbrenneman]

What EGL_NATIVE_VISUAL_ID is depends entirely on the platform. With EGL_KHR_platform_gbm, it's a fourcc code, but with EGL_KHR_platform_x11, it's the XID for an X Visual.

EGL_KHR_platform_wayland and EGL_EXT_platform_device leave it undefined, so all you can assume is that it's some unique, driver-specific 32-bit value.

[stonesthrow]

Thanks. I'm sure we had this discussion. But that was many context switches ago.

[fooishbar]

If this is the only concern causing it to not be usable with eglChooseConfig(), we can simply reserve it upstream and ensure that it never gets used. (Practically speaking it is of zero concern since the upstream FourCC codes are defined in terms of ASCII character codes, and ASCII 255 is not going to pass review, but we can also explicitly reserve it if it would make you more comfortable.)

[kbrenneman]

I figured that the chance of 0xFFFFFFFF being a real format is pretty much zero, but unless it's explicitly reserved, I wouldn't want to count on it never being used for some special meaning.

If we do reserve it upstream, though, then selecting a format with eglChooseConfig should work fine.

[cubanismo]

Agreed, explicitly reserving 0xFFFFFFFF as a special format token in drm_fourcc.h is probably the cleanest way to resolve this. I was a bit disappointed with the prior resolution.

[amshafer]

Since the consensus is to limit this to EGLStreams, should we still consider going ahead with reserving this upstream? Or should we not bother and stick with the current "reduced/streams-focused" behavior of this extension which doesn't worry about specifying this in eglChooseConfig()? It seems like the later, but I figure we should explicitly decide that.

[cubanismo]

I still think it'd still be best to explicitly reserve 0xFFFFFFFF in drm_fourcc.h. Probably best to propose that upstream before merging this. I don't know why anyone would object, but you never know.

[fooishbar]

Modifiers are essentially required for any interop, so this extension should include both. DRM_FORMAT_MOD_INVALID is the 'I don't know what the internal layout is, figure it out under the hood' escape hatch for drivers which do not support modifiers.

[kbrenneman]

The same EGLConfig could correspond to multiple possible modifiers, possibly even different modifiers for different buffers in the same EGLSurface, so an EGLConfig attribute wouldn't work.

In the case of a stream, EGL_NV_stream_consumer_eglimage can let you specify a format modifier for the resulting EGLImage, but it might use a different modifier for the EGLSurface and then convert from one to the other in eglSwapBuffers.

[fooishbar]

Sure. On the other hand, there's no reason for an EGLConfig to be 1:1 with a format, either: if you have 8/8/8/8 colour channel sizes, that could be ARGB8888 or ABGR8888 ... so one possible resolution would be to get a list rather than a single value.

[cubanismo]

I think part of the point of this extension is to assert that for a given EGLDisplay, there is a 1:1 mapping between a given EGLConfig and a DRM format, which is different than generally trying to map 4 8-bit channels to a unique format. The latter is what necessitates this extension in the first place. Are there cases where such an assertion would be problematic? The only thing I can think of is the EGL_EXT_present_opaque extension, but the implementation of that is more like a mutable surface format/pixel aliasing thing than actually exposing two separate formats for the surface. I.e., exposing two DRM formats as an attempt to expose some implementation detail of that extension is at odds with the purpose of that extension.

It may make sense to query a list of modifiers compatible with an EGLConfig, but it would need a dedicated command, possibly multiple commands. I'm also not sure an EGLConfig is sufficient info to enumerate modifiers. Different usage of the config could impose different limitations on the modifiers available.

[fooishbar]

I think part of the point of this extension is to assert that for a given EGLDisplay, there is a 1:1 mapping between a given EGLConfig and a DRM format, which is different than generally trying to map 4 8-bit channels to a unique format.

Sure, that is pretty easily implementable. Practically speaking, it's what Mesa does anyway to be honest.

It may make sense to query a list of modifiers compatible with an EGLConfig, but it would need a dedicated command, possibly multiple commands. I'm also not sure an EGLConfig is sufficient info to enumerate modifiers. Different usage of the config could impose different limitations on the modifiers available.

That's very true. Generally you express the superset of what you could do, then since you have a list anyway, filter them down later.

But that's what makes me kind of confused about this extension: what does it enable? For interop with anything non-Streams - KMS, VA-API, V4L2, Wayland, etc - it needs to be able to handle modifiers. The description notes that EGLStreams can use it to pass a format and modifiers to external consumers, but without any modifiers here, you'd already need to pull the actual modifier (as well as pass a set of acceptable modifiers for negotiation) in through some other extension?

If it's just Streams-only, then I think it makes sense to scope this extension to Streams only. If it's meant to be for wider use, then I think I'd need to understand more about the envisioned usecases to say something intelligent.

[cubanismo]

Yeah, I think you're right there is an assumption regarding modifiers here right now, and streams happen to be the only drawable type you can create (and only sort of) with modifiers right now. It's probably fair to share a bit about about the broader plans this emerged from:

• We have no plans to write further extensions that allow creating window/pbuffer/pixmap surfaces with explicit drm_fourcc codes or modifiers or some external image allocation mechanism, so perhaps this extension will never be interesting on those surface types anyway.

• We do indeed need to query modifiers from elsewhere to create EGLImage-based stream consumers. EGLStreams originally were designed to do something like a superset of the format modifier negotiation logic internally: You connect a single producer and consumer (later extensions added many:many mappings), and they do implementation-specific stuff to negotiate exact formats and layouts. The EGLImage consumer replaces the consumer side of that negotiation with format modifiers because an EGLImage, unlike other consumer types, doesn't imply any particular usage. Instead, the user of the images has to go query format modifiers from the things it intends to use the images with. The consumer still defers to the producer for actual format selection though, so the user needs some way to ask the producer what format it's going to use to be able to fetch appropriate modifiers. Hence, this extension currently closes the loop on a workflow specific to the EGLStreams image consumer. This is also why querying the modifiers from the EGLConfig isn't necessary here: They're effectively automatically queried internally when constructing the stream, just like they always have been. Again, very specific to streams.

• We're exploring the idea of writing another extension to remove the other half of the streams usage, the producer side, by introducing another surface type that is essentially a bag of images. If that exists, you do need a way to query some format modifiers, but in our current designs, it is just assumed the surface supports whatever modifiers an EGLImage in general supports. EGLImages are constructed independently and then a surfaces is constructed from those, so you just use the existing EGLImage query logic to get your modifiers. That said, you still need a way to correlate formats to EGLConfigs to ensure you create compatible images, so this extension is still needed there. Arguably, you probably don't really need the EGLConfig at all in that use case. You can just query the supported formats for dma-bufs as well. However, it turns out you still want it to specify the auxiliary and non-color buffers and stuff like that, and the API interactions get weird if you pull EGLConfigs out of surface creation. So again, a way to tie an EGLConfig to a DRM fourcc code is useful, but a format modifier config attribute is a bit redundant, unless you tried to make it more specific to the usage somehow, but I'm not sure what extra info we could report absent introducing a large universal allocator-like API into EGL, which probably still isn't the right way to solve that whole mess.

I think the conclusion this leads me to is that it's probably fine to limit this to streams for now, and add language in any potential future bag-of-images surface extensions to say "We can do that too," and let EGL window/pixmap/pbuffer surfaces avoid this for now. The only thing I'd want to be careful about is ensuring we can still advertise this for EGLConfigs that expose those surface type bits as long as they also expose the stream surface producer bit so we don't have to double up our config list just to expose this attrib.

We also aren't interested in forcing EXTs through without approval of others. If you think this should be NV-only and aren't interested in further discussing changes, we could make it NV-only. However, I think it is something that may be generally useful if others want to adopt or develop any of the above extension ideas, and we discussed internally and decided we can kludge around the lack of this extension well enough for now to avoid schedule pressure, so it would be nice to reach consensus on an EXT design if there's interest. If you envision a use case beyond those above where including a format modifier query makes sense, or of course if there are holes in my logic above, we can work out a way to add a similar query in. I imagine it's probably just a new config attrib query that returns an array. eglGetConfigAttrib() almost works for that as-is, but you'd have to kludge around and add more attributes to define and negotiate the length of the array, so it's probably better to just have a new command.

[kbrenneman]

If this is limited to streams, then is there any use beyond EGL_NV_stream_consumer_eglimage?

For the bag-of-images EGLSurface idea, adding a fourcc code as an EGLConfig attribute would be reasonably clean -- it's a new surface type, so there's no prior behavior to change or assumptions to break.

[fooishbar]

Thanks a lot for the background James! That makes a lot of sense. I think that restricting it to Streams is a good idea: I'm definitely worried about unintended consequences with opening it up more widely, so if there's no usecase driving us to expose it more widely, then I'd prefer to keep it restricted until one did turn up. Given that, and that you've throught through all the interactions with Streams, I don't think there's much point trying to work with modifiers etc.

[oddhack]

@stonesthrow this is marked Approved to Merge, but there's still an outstanding change requested by @fooishbar . Can we clear that block before I hit the merge button?

[fooishbar]

@oddhack i want to discuss this further to make it EXT. @cubanismo @kbrenneman if this is time-sensitive then I would prefer it be scoped to Streams only

[amshafer]

Thanks everyone for the feedback, I updated with a new version that specifies this is limited to EGLConfigs which have the EGL_STREAM_BIT_KHR bit set. Does that look like what you all had in mind? I added a very short summary of the discussion in the Issues section to document why it was considered.

Any other changes I should make?

[kbrenneman]

Is a fourcc code meaningful for any consumer type other than EGLImages?

[cubanismo]

Is a fourcc code meaningful for any consumer type other than EGLImages?

I don't think so, but this extension operates on the producer side, so I don't think it should make assumptions about the consumer.

[kbrenneman]

I don't think so, but this extension operates on the producer side, so I don't think it should make assumptions about the consumer.

Tue. I'll admit, putting the fourcc format on the producer side when the format modifiers are on the consumer side does bother me a bit, but I can't think of any other way to do that. If we added EGL_LINUX_DRM_FOURCC_EXT as an attribute for eglCreateImage, then we'd still need something to query a DRM format for the producer so that the color depth can match, and to avoid unnecessary blits.

Anyway, mostly I was wondering whether this should be an EXT extension if it only makes sense in the context of an NV extension.

[fooishbar]

I think the Streams ext now needs to be added to the dependencies section, but the rest lgtm, thanks!

[amshafer]

Updated, thanks!

[cubanismo]

Anyway, mostly I was wondering whether this should be an EXT extension if it only makes sense in the context of an NV extension.

I think as long as no one objects, EXT is preferable to avoid potential awkwardness of any future EXT users depending on an NV. The mechanism seems