l1npengtul / nokhwa

Cross Platform Rust Library for Powerful Webcam/Camera Capture
Apache License 2.0
482 stars 111 forks source link

Wrong `frame_rate` on Windows #144

Open Giotino opened 10 months ago

Giotino commented 10 months ago

While my webcam offers multiple framerate options for the MJPEG format: CameraFormat { resolution: Resolution { width_x: 1920, height_y: 1080 }, format: MJPEG, frame_rate: 60 } CameraFormat { resolution: Resolution { width_x: 1920, height_y: 1080 }, format: MJPEG, frame_rate: 30 } and many more.

The library select the one that best fit the request (as expected), I can visually confirm it's running at 60 FPS when I require 60, but it reports always frame_rate: 1 (that is not offered by my webcam)

let mut camera = Camera::new(index, requested).unwrap();
println!("Camera format: {:?}", camera.camera_format());

CameraFormat { resolution: Resolution { width_x: 1920, height_y: 1080 }, format: MJPEG, frame_rate: 1 }

Giotino commented 10 months ago

The positions of the numerator and denominator of MF_MT_FRAME_RATE (https://learn.microsoft.com/en-us/windows/win32/medfound/mf-mt-frame-rate-attribute) seem to be different between SetCurrentMediaType and GetCurrentMediaType.

With this ugly byte swap I get the correct frame_rate

let mut bytes: [u8; 8] = unsafe { media_type.GetUINT64(&MF_MT_FRAME_RATE) }
    .unwrap()
    .to_ne_bytes();
bytes.swap(0, 4);
bytes.swap(1, 5);
bytes.swap(2, 6);
bytes.swap(3, 7);
let frame_rate = u64::from_ne_bytes(bytes);
println!("Frame rate: {}", frame_rate as u32);
Giotino commented 10 months ago

There is something strange going on, it seems that Windows is trying to correct the endianness of the two u32 (numerator and denominator).

With the "wrong" endianness (the one used by the library):

Bytes IN  SetCurrentMediaType: [0, 0, 0, 1, 0, 0, 0, 60]
Bytes OUT GetCurrentMediaType: [1, 0, 0, 0, 60, 0, 0, 0]

With the "correct" endianness:

Bytes IN  SetCurrentMediaType: [1, 0, 0, 0, 60, 0, 0, 0]
Bytes OUT GetCurrentMediaType: [1, 0, 0, 0, 60, 0, 0, 0]
Giotino commented 10 months ago

I also fear that since the framerate is represented as numerator/denominator there could be devices that use framerates like 29.97 that can be represented as 2997/1000 and nokhwa is going to read 2997 as the framerate.

Giotino commented 10 months ago

I just found out this issue was already reported #110 and #139

Giotino commented 10 months ago

I think I get what's happening: nokhwa is using the wrong endianness for the two u32 that represents the numerator and denominator, since https://github.com/l1npengtul/nokhwa/blob/dbdb42bdc6c377b029cf40b3cca02c0a76cedf53/nokhwa-bindings-windows/src/lib.rs#L1027-L1033 it's writing the two u32 in big-endian inside the u64: [0, 0, 0, 1, 0, 0, 0, FRAMERATE] instead of [1, 0, 0, 0, FRAMERATE, 0, 0, 0]. Then Windows converts it to [1, 0, 0, 0, FRAMERATE, 0, 0, 0] (I don't know why, but it's trying to fix this mistake). In the end Windows use the correct endianness when answering to GetCurrentMediaType and nokhwa read it wrongly (it reads the denominator instead of the numerator).

The fix required to make it works is to review the way that these number are built and parsed, also beacause currently it can't support more than 255 FPS. This also includes the part of code that build the list of available formats.

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)