BITMAPINFOHEADER biCompression flag inconsistencies

[mdepot]

The BITMAPINFOHEADER structure contains a DWORD biCompression field. There seems to be a number of closely related issues with how this field is defined in win32metadata.

First of all, the biCompression field is defined inconsistently, in three different ways. In System.Drawing.NativeMethods it's an int, while in Windows.Win32.DirectShow it's a uint, and finally in Standard it's an enumeration of type BI.

This page https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader says the biCompression field is supposed to be able to hold either a FOURCC code (for compressed formats) or a number of defined flags (for uncompressed formats). And this page https://learn.microsoft.com/en-us/windows/win32/directshow/fourcc-codes says a FOURCC code is a 32-bit unsigned integer.

There is also a problem with the way the BI_* flag values meant to be assigned to the biCompression are defined. MS.Win32.NativeMethods has a single BI_* constant defined (BI_RGB), and is missing at least the BI_BITFIELDS flag. However System.Drawing.NativeMethods has it's own BI_* constants, this time for both BI_RGB and BI_BITFIELDS. And finally, the BI enumeration in Standard only has RGB defined. Based on this page https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-wmf/4e588f70-bd92-4a6f-b77f-35d0feaf7a57 from the WMF docs, it looks as if there are nine possible values for this that should be defined.

It would seem that all three definitions of biCompression should at least be consistent with each other, and perhaps they should also all be based on a more fully flushed out uint based BI enumeration as well.

Without at least fixing the type consistency, a projection may allow for assigning the int based BI_RGB to the uint biCompression field defined in Windows.Win32.DirectShow, resulting in a type mismatch error, which is how I found this.

[mikebattista]

We have no control over .NET definitions here. Is the win32metadata representation correct?

Is the main problem that FOURCC codes effectively represent an open set, so using a closed set enum at all for this field is not appropriate?

[mdepot]

Is the main problem that FOURCC codes effectively represent an open set, so using a closed set enum at all for this field is not appropriate?

What I had initially ran into was the int versus uint type mismatch. However that led me to notice these fields were inconsistently defined. Since I'm just working at the level of the projection I'm not sure how far upstream this should be addressed.

[mikebattista]

@AArnott what are your thoughts on this? From the report, it sounds like .NET types we don't control are incorrect, and the metadata we publish is correct.

[AArnott]

In System.Drawing.NativeMethods it's an int,

Is that a reference to private code in the runtime? Matching that is not a goal, nor is it expected to present a conflict to the .NET user because they don't have access to that class.

while in Windows.Win32.DirectShow it's a uint

If that's a reference to win32metadata, I cannot find that namespace or type. I'm looking at 40.0.14-preview of the metadata.

and finally in Standard it's an enumeration of type BI.

What is "Standard"? Can you provide the library and full API name?

Ultimately though, the win32metadata's goal is to match the headers, with a little semantic richness added (e.g. add enums where appropriate). Since the headers define biCompression to be a DWORD, the metadata should use uint or an enum of the same length.
In this case however, as you point out that this field may be used for FOURCC codes, I believe the field should be left as a uint instead of an enum.

@mikebattista I notice in the documentation the reference to BI_JPG as a constant, and it's interesting to note that the metadata defines BI_COMPRESSION as an enum with value BI_JPEG but not BI_JPG. Why the extra E?
We should probably review the other uses of this enum type (there are 3, evidently) to see if they also should be changed back to uint if they can similarly store FOURCC codes.

[mdepot]

What is "Standard"? Can you provide the library and full API name?

What I had done to find these values:

• Run ILSpy
• Loaded microsoft.windows.sdk.win32metadata.10.0.19041.5-preview.1\content\Windows.Win32.winmd
• Searched for biCompression
• Ignored the results that did not seem directly related to BITMAPINFOHEADER

That left me with these remaining results:

System.Drawing.NativeMethods+BITMAPINFOHEADER.biCompression :

// System.Drawing.Common, Version=6.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
// System.Drawing.NativeMethods.BITMAPINFOHEADER
public int biCompression;

Windows.Win32.DirectShow.BITMAPINFOHEADER.biCompression :

// Windows.Win32.winmd, Version=10.0.19041.5, Culture=neutral, PublicKeyToken=null
// Windows.Win32.DirectShow.BITMAPINFOHEADER
public uint biCompression;

Standard.BITMAPINFOHEADER.biCompression :

// PresentationFramework, Version=6.0.2.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35
// Standard.BITMAPINFOHEADER
public BI biCompression;

• Then I clicked on the BI in the last result to jump to that definition, which led to

Standard.BI :

// PresentationFramework, Version=6.0.2.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35
// Standard.BI
internal enum BI
{
	RGB
}

Apologies if this is not the best way to look into this, I was attempting to follow the advice in the README.md but I'm not familiar with this utility.

[ChrisDenton]

In the latest metadata it's an enum:

// Windows.Win32.Graphics.Gdi.BITMAPINFOHEADER
public BI_COMPRESSION biCompression;

// Windows.Win32.Graphics.Gdi.BI_COMPRESSION
public enum BI_COMPRESSION
{
	BI_RGB,
	BI_RLE8,
	BI_RLE4,
	BI_BITFIELDS,
	BI_JPEG,
	BI_PNG
}

EDIT: There is also KS_BITMAPINFOHEADER, which according to the docs is the same as BITMAPINFOHEADER but it's defined differently in the metadata:

// Windows.Win32.Media.KernelStreaming.KS_BITMAPINFOHEADER
public uint biCompression;

[riverar]

@mikebattista I'm not familiar with the semantics chosen for metadata when it comes to parameters with enumeration types. If it is meant to convey to consumers that there is a closed set of values then yes, we should change this back to a uint. If that's not the case, then the enumeration is fine (but needs a tweak to inherit from uint).

I ask because C# allows for enum Foo { A, B, C }; Foo foo = (Foo)42.

[mdepot]

Because of the ability of this field to hold FOURCC values, it appears to be an open set. Thus it would seem that it should be defined as public uint biCompression rather than as an enum.

However, there are also predefined values that would have normally been an enum if it were a closed set. Because of this would it make sense to ALSO define the constants that normally would have been an enum, but just define them as uints? In this way the values will be readily available for assignment by name while still keeping it an open set. For example:

// Proposed addition
public const uint BI_RGB = 0x0000;
public const uint BI_RLE8 = 0x0001;
public const uint BI_RLE4 = 0x0002;
public const uint BI_BITFIELDS = 0x0003;
public const uint BI_JPEG = 0x0004;
public const uint BI_PNG = 0x0005;
public const uint BI_CMYK = 0x000B;
public const uint BI_CMYKRLE8 = 0x000C;
public const uint BI_CMYKRLE4 = 0x000D;

(Values based on https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-wmf/4e588f70-bd92-4a6f-b77f-35d0feaf7a57)

[kennykerr]

At least for C++ and Rust you can use an enum here and still support an open-ended set by placing a different value into the enum. And it seems C# supports this as well. #1442 (comment)

[AArnott]

@kennykerr I'm glad you spoke up, because I was about to say I thought rust required enums to be a closed set.
I thought our policy was to avoid enums when it's an open set. I think that fed into why we represent HRESULT as a struct and Win32Error as an enum.

[AArnott]

@mdepot When you open the metadata in ILSpy, ILSpy will automatically open a bunch of reference assemblies as well. Most of the search results you shared come from other assemblies, which are irrelevant as they are private to those assemblies and win32metadata does not attempt to match them.

[kennykerr]

I think that fed into why we represent HRESULT as a struct and Win32Error as an enum.

Yes, I don't really see the value in that distinction. It's all the same to me and I end up having to normalize them on my end where everything ends up open-ended and represented the same way.

[ChrisDenton]

In ILSpy, you can go to File -> Manage Assembly Lists and start a new, empty, list. Then just add the files you want. This should hide other results when searching.

[mikebattista]

I'm fine keeping this as an enum. In which case, what are the pending changes? Just also applying the enum to KS_BITMAPINFOHEADER? Does the enum contain all the values it should?

[riverar]

I submitted a PR to return it to uint to simplify matters (#1450).

I don't think we can assume all languages (current or future) will support open-ended enums. For example, I haven't used Dart much but I couldn't quite figure out how to make this work nicely over there. (Maybe Dart's enhanced enums would come into play here?)

Duplicate discussion: #261

[kennykerr]

cc @timsneath

[timsneath]

From a Dart perspective, I'm fine with this being an enum, although I'd love to see some metadata marker to identify enums that are open-ended. Enhanced enums could work, but I want to explore using the new inline class feature in Dart 3 for this kind of value.

[mdepot]

I don't think we can assume all languages (current or future) will support open-ended enums.

Thank you for highlighting this. I am currently using this projection for Go. In Go the common way to handle enums (today) is to define a custom type, and then create constants of that type representing the possible values. There is a proposal to enhance how enums work in Go, but that's still a work in progress. While it may be possible to define more constants of that type later, doing so would split the definition of possible values into different areas of code maintained by different people.

[mikebattista]

In general we have used enums for closed sets which follows the .NET design guidelines. The conclusion of #261 was not to use an enum for that reason (though somewhere along the way looks like an enum was mapped to this open set field).

I'm open to using enums for open sets if there's a proposal that works for all languages.

[ChrisDenton]

Is there an attribute that could be used/repurposed to indicate whether a set is open or closed? Then projections who treat them all as open can simply ignore it whereas those who want to do something special can do so.

[mikebattista]

Not currently but we could create one if that would work for everyone.

If we did that, I would say that absence of the attribute indicates closed, and presence of the attribute indicates open.

[mdepot]

I like the idea of having the attribute, but a single open/closed flag would only indicate two use cases. We actually have three. The simple cases are a closed set with a defined set of known values for the enumeration, and open set with no defined set of values at all. Then there is this third middle ground case of an open set that additionally has a number of known values defined that can be assigned.

Before deciding on how this new attribute should be implemented, it may make sense to first decide on how this third use case should be handled. Above I proposed not using an enum but also defining any known members of the open set as constants. Does it make sense for that to be the standard for all instances of this third case scenario? Are there alternatives?

I think only after this is decided upon does it make sense to design how a new attribute should work, as it's likely dependent on how this third case is to be handled,

[AArnott]

and open set with no defined set of values at all

We have never used enums for this. We use typedefs for some of these (where the native code does).

[mdepot]

On thinking about this further, I can see that having the typedef plus an open/closed set flag would be enough for a projection to decide if it should actually implement the set with an enum or not in the projection's target language. As a simple user of a projection, the responsibility of whether that decision should happen in the metadata or the projection was not clear to me at first.

[mikebattista]

What should the attribute be named? [OpenSetEnum]?

[riverar]

In the interest of keeping things simple and moving, I vote for just correcting biCompression (uint) and let the Windows provided FOURCC values be constants.