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

Possible misbinding #1957

Open
marci1175 opened this issue Aug 6, 2024 · 5 comments
Open

Possible misbinding #1957

marci1175 opened this issue Aug 6, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@marci1175
Copy link

marci1175 commented Aug 6, 2024

Summary

I belive I have found a misbinding in the Win32_Media_MediaFoundation feature.
The IMFSourceReader::SetCurrentMediaType(arg , . . .) expects arg to be of type u32. The documentation provided my microsoft says there are 3 variants of arg:
0–0xFFFFFFFB | The zero-based index of a stream.
MF_SOURCE_READER_FIRST_VIDEO_STREAM 0xFFFFFFFC | The first video stream.
MF_SOURCE_READER_FIRST_AUDIO_STREAM 0xFFFFFFFD | The first audio stream.

But in the create all of them have an i32 type. Which can still be casted to a u32, but it was confusing for me at first and I dont think this is intended behavior.

Crate manifest

[dependencies]
anyhow = "1.0.86"

[dependencies.windows]
version = "0.58.0"
features = [
    "Data_Xml_Dom",
    "Win32_Foundation",
    "Win32_Security",
    "Win32_System_Threading",
    "Win32_UI_WindowsAndMessaging",
    "Win32_Media_MediaFoundation",
]

Crate code

use windows::{core::w, Win32::Media::MediaFoundation::{MFCreateMediaType, MFCreateSourceReaderFromURL, MFMediaType_Video, MFStartup, MFVideoFormat_RGB32, MFSTARTUP_FULL, MF_MT_MAJOR_TYPE, MF_MT_SUBTYPE, MF_SOURCE_READER_FIRST_VIDEO_STREAM, MF_VERSION}};


pub unsafe fn mf_startup() -> anyhow::Result<()> {
    Ok(MFStartup(MF_VERSION, MFSTARTUP_FULL)?)
}

fn main() -> anyhow::Result<()> {
    unsafe {
        mf_startup()?;

        let source_reader = MFCreateSourceReaderFromURL(w!("video://0"), None)?;

        let media_type = MFCreateMediaType()?;

        media_type.SetGUID(&MF_MT_MAJOR_TYPE, &MFMediaType_Video)?;
    
        media_type.SetGUID(&MF_MT_SUBTYPE, &MFVideoFormat_RGB32)?;
    
        source_reader.SetCurrentMediaType(MF_SOURCE_READER_FIRST_VIDEO_STREAM, None, &media_type)?; //Compiler error
  
    }

    Ok(())
}
@marci1175 marci1175 added the bug Something isn't working label Aug 6, 2024
@kennykerr
Copy link
Contributor

Looks like the Win32 metadata invents the MF_SOURCE_READER_CONSTANTS enum. I'm not sure where that comes from. The type of the constants like MF_SOURCE_READER_FIRST_VIDEO_STREAM, which should just be loose constants, is u32 in the original API definitions but ends up i32 in metadata. I'll transfer to the Win32 metadata repo for consideration.

@kennykerr kennykerr transferred this issue from microsoft/windows-rs Aug 6, 2024
@tim-weis
Copy link
Contributor

tim-weis commented Aug 6, 2024

The enum comes from mfreadwrite.idl that's compiled to mfreadwrite.h. If this was an integer literal 0xFFFFFFFD (for example) should be of type unsigned int (as I understand the rules), though I don't know whether the enum changes things.

@riverar
Copy link
Collaborator

riverar commented Aug 6, 2024

using T = std::underlying_type<__MIDL___MIDL_itf_mfreadwrite_0000_0001_0001>::type;
std::cout << typeid(T).name() << std::endl;
> int

In C and, the underlying type is always int. MSVC C++ also assumes the underlying type is int, see also C4471.

@kennykerr
Copy link
Contributor

The issue is more about what the API expects than the limitations of the C and C++ languages. The API expects unsigned arguments and C/C++ compilers were happy to smooth things over. Same with #define constants like ERROR_ACCESS_DENIED that are used with GetLastError and SetLastError and expect unsigned values.

@mikebattista
Copy link
Contributor

This enum is not an invented enum. It's scraped from the headers so the type is whatever ClangSharp detects. We just give it a friendly name based on the recommendation from ClangSharp.

enum __MIDL___MIDL_itf_mfreadwrite_0000_0001_0001
{
MF_SOURCE_READER_INVALID_STREAM_INDEX = 0xffffffff,
MF_SOURCE_READER_ALL_STREAMS = 0xfffffffe,
MF_SOURCE_READER_ANY_STREAM = 0xfffffffe,
MF_SOURCE_READER_FIRST_AUDIO_STREAM = 0xfffffffd,
MF_SOURCE_READER_FIRST_VIDEO_STREAM = 0xfffffffc,
MF_SOURCE_READER_MEDIASOURCE = 0xffffffff
} ;

We've had feedback in the past that we shouldn't deviate from the original types as scraped by ClangSharp. If we add [AssociatedEnum] on the parameter where these constants are used, would the projections handle things appropriately?

@mikebattista mikebattista self-assigned this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants