The generator is a mess about drm-fourcc-rs

[Drakulix]

Thanks for the feedback, a couple of remarks:

DrmFourcc implements TryForm, while DrmModifier uses From with an additional Unrecognized(u64) variant, making it inconsistent

This is by design, given a large portion of modifiers are expressed as marcos in the original header, there is no way to ensure this crate contains all modifiers. The Unrecognized variant is basically required (and would have to be implemented by downstream otherwise) to make any use of it.

There is also struct UnrecognizedModifier(pub u64), which I don't get the point of, since we have DrmModifier::Unrecognized(u64)

I think that is indeed just an oversight of previous implementations

Many implementations of PartialEq, Eq that could have been derives

There are two for DrmModifier, which are there again by design, because some modifiers share the same internal code (but have multiple names).
For those the equality test needs to be true, even though they are different variants.

All enum conversion can be implemented by num_enum

I don't think (please correct me on that one) that num_enum can handle multiple names for the same value (though it can handle the reverse).
This might be true for DrmFormat, but given we generate the conversions anyway, I am not sure there is much to gain here (except another dependency).

Crate can be fully no_std with optional alloc for String

True. Pull requests welcome

Missing Into, From, TryFrom implementations where it makes sense

Not sure, which ones you are referring to, but again PRs welcome :)

I could volunteer for a refactoring, but I feel that it would be a bomb PR with a complete rewrite, which I'm not sure is welcome

I certainly would prefer multiple PRs with smaller scope to make review easier. But as long as if stuff is clearly improved, I will put it the time to try and deal with a large PR as well.

But please re-evaluate your points. Some stuff is there by design, because drm modifiers are a mess (like this crate).

[dzfranklin]

FYI I built this for a prototype which I ended up abandoning. I've added Drakulix as a contributor, and I'm happy to do any admin work needed, but I have no interest in refactoring/cleaning up.

[chemicstry]

Not to be rude, but this whole crate is a bit of a mess:

• DrmFourcc implements TryForm<u32>, while DrmModifier uses From<u64> with an additional Unrecognized(u64) variant, making it inconsistent
• There is also struct UnrecognizedModifier(pub u64), which I don't get the point of, since we have DrmModifier::Unrecognized(u64)
• Many implementations of PartialEq, Eq that could have been derives
• All enum conversion can be implemented by num_enum
• Crate can be fully no_std with optional alloc for String
• Missing Into, From, TryFrom implementations where it makes sense

I could volunteer for a refactoring, but I feel that it would be a bomb PR with a complete rewrite, which I'm not sure is welcome

[chemicstry]

Thanks for the in depth reply, I take some of my points back

DrmFourcc implements TryForm, while DrmModifier uses From with an additional Unrecognized(u64) variant, making it inconsistent

This is by design, given a large portion of modifiers are expressed as marcos in the original header, there is no way to ensure this crate contains all modifiers. The Unrecognized variant is basically required (and would have to be implemented by downstream otherwise) to make any use of it.

Makes sense, hence I would like to suggest the same for DrmFourcc, since it could also be missing formats that are defined in fourcc, but are not yet in the kernel. I.e. MJPG is already used by libcamera in raspberry pi, but is not yet upstreamed to linux kernel. This would allow using the library without forking it when format is missing.

After looking at drm_fourcc.h it seems that it is more cursed than I thought

[Drakulix]

Makes sense, hence I would like to suggest the same for DrmFourcc, since it could also be missing formats that are defined in fourcc, but are not yet in the kernel. I.e. MJPG is already used by libcamera in raspberry pi, but is not yet upstreamed to linux kernel. This would allow using the library without forking it when format is missing.

Oh interesting, I always assumed drm_fourcc.h would contain all valid formats on a given platform+distro, but in that case we have a definite use-case for an Unrecognized variant of the Fourcc enum.

After looking at drm_fourcc.h it seems that it is more cursed than I thought

That is how I felt contributing to this generator. 😆