Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FourCC: Improve debug prints #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dcz-self
Copy link

@dcz-self dcz-self commented Oct 6, 2024

This looks bad:

FourCC {
                repr: [
                    89,
                    85,
                    89,
                    86,
                ],
            },

This looks better:

FourCC("YUYV"),

Fallback for non-ASCII:

FourCC(01 ff 20 cd)

I think drm_fourcc is a better choice for representing fourcc in Linux, but I'm not ready to propose such a change.

@MarijnS95
Copy link
Collaborator

I think drm_fourcc is a better choice for representing fourcc in Linux, but I'm not ready to propose such a change.

I had once done a conversion to drm_fourcc but never ended up PR'ing it because that crate, as its name implies, was only ever designed to hold enum variants for the DRM crate, and enum variants cannot be extended externally. Quite a bunch of the V4L2_PIX_FMT_* enums defined at https://elixir.bootlin.com/linux/master/source/include/uapi/linux/videodev2.h are not represented in it currently, and I don't know if we can request the drm_fourcc maintainers to include the V4L2 types. Because of that, DrmFourcc is lossy and any conversion for an unknown fourcc code will result in an UnrecognizedFourcc error/carrier.

@dcz-self
Copy link
Author

I just asked dzfranklin/drm-fourcc-rs#25

@MarijnS95
Copy link
Collaborator

@dcz-self I think you pushed some of your own testing to the master branch, which was used as a base branch for this PR.

To not have to close this PR and lose all context, perhaps push that to a separate branch :)

@dcz-self
Copy link
Author

dcz-self commented Nov 1, 2024

Oops. Thanks for noticing.

Anyway, is there any interest in this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants