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.

Sign up for GitHub

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

Jump to bottom

Fix frame_rate on Windows #149

Draft
wants to merge 1 commit into
base: senpai
Choose a base branch
from
Draft

Fix frame_rate on Windows #149

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions nokhwa-bindings-windows/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,14 @@ pub mod wmf {
};

let frame_rate = match unsafe { media_type.GetUINT64(&MF_MT_FRAME_RATE) } {
Ok(fps) => fps as u32,
Ok(fps) => {
let mut numerator = (fps >> 32) as u32;
let denominator = fps as u32;
if denominator != 1 {
numerator = 0;
}
numerator
Comment on lines +972 to +975
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As fare as I understand the API https://learn.microsoft.com/en-us/windows/win32/medfound/mf-mt-frame-rate-attribute the numerator should be divided by the denominator to get the frame rate.

Suggested change
if denominator != 1 {
numerator = 0;
}
numerator
if denominator != 0 {
numerator / denominator
} else {
0
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but as you can see here https://github.com/Giotino/nokhwa/blob/senpai/nokhwa-bindings-windows/src/lib.rs#L599-L636 Nokhwa doesn't support non integer framerates and simply reports 0 when there is one.
And, since the framerate is an integer, it's gonna report a wrong number if we use your approach, which is not to be ignored, but we should agree that framerate is not really supported by the device.

This was also written in my original issue (#144 (comment)):

Also nokhwa should keep in mind that there might be some framerates that are not X/1 (like 2997/1000), I think here we have 2 solutions that doesn't require to rewrite the frame_rate handling:

  1. Return an error when the framerate is not X/1
  2. "Transform" A/B -> X/1 rounding X (es. 2997/1000 -> 30/1)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a rounded framerate is better than no frame rate

},
Err(why) => {
return Err(NokhwaError::GetPropertyError {
property: "MF_MT_FRAME_RATE".to_string(),
Expand Down Expand Up @@ -1027,8 +1034,8 @@ pub mod wmf {
let fps = {
let frame_rate_u64 = 0_u64;
let mut bytes: [u8; 8] = frame_rate_u64.to_le_bytes();
bytes[7] = format.frame_rate() as u8;
bytes[3] = 0x01;
bytes[4] = format.frame_rate() as u8;
bytes[0] = 0x01;
u64::from_le_bytes(bytes)
};
let fourcc = frameformat_to_guid(format.format());
Expand Down
Loading