Skip to content

Fix UVC probe and commit on MacOS #1820

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

Merged
merged 1 commit into from
Jan 7, 2023
Merged

Fix UVC probe and commit on MacOS #1820

merged 1 commit into from
Jan 7, 2023

Conversation

Staacks
Copy link
Contributor

@Staacks Staacks commented Dec 27, 2022

Describe the PR
This is a minor change to the USB Video class implementation to accept a too small wLength during probe and commit requests in order to fix MacOS compatibility.

Additional context
My (UVC project)[https://github.com/Staacks/gbinterceptor] as well as a minimal example on a Raspberry Pi Pico based on the examples here fails on MacOS during SET CUR Probe because wLength is set to 0x22 instead of the expected 0x30. This behavior can actually be observed with WireShark on MacOS for any commercial webcam that I plugged in and it is marked by Wireshark as a malformed package.

However, if I understand section 4.1 of the UVC 1.5 definition correctly, this behavior is allowed:

If the parameter block is longer than is indicated in the wLength field, only the initial bytes of the
parameter block are returned. If the parameter block is shorter than is indicated in the wLength
field, the device indicates the end of the control transfer by sending a short packet when further
data is requested.

But even if I misinterpret this (this is the first time I really look into these specs) and it is a bug by Apple, I still recommend to accept too small wLength values here as I do not see any relevant downside to doing so.

@kkitayam
Copy link
Collaborator

Thank you for the PR.

My (UVC project)[https://github.com/Staacks/gbinterceptor] as well as a minimal example on a Raspberry Pi Pico based on the examples here fails on MacOS during SET CUR Probe because wLength is set to 0x22 instead of the expected 0x30.

I guess MacOS runs as a UVC 1.1 host. UVC 1.1 spec. shown below
image
And UVC 1.5 spec. shown below.
image

However, if I understand section 4.1 of the UVC 1.5 definition correctly, this behavior is allowed:

It seems to be the description for Get Request, not for Set Request. SET CUR is a Set Request. So, I guess the behavior does not apply to SET CUR.

But even if I misinterpret this (this is the first time I really look into these specs) and it is a bug by Apple, I still recommend to accept too small wLength values here as I do not see any relevant downside to doing so.

Yes, that's right. I think it is better to be able to run on a actual machine than to strictly comply with the specifications.

I will check the PR.

@kkitayam
Copy link
Collaborator

I have checked that there are no side-effects. No problem.
On macOS, 6 bytes following UVC 1.1 data member are not initialized by the host. However, the current implementation does not use these data.

@hathach
Copy link
Owner

hathach commented Jan 7, 2023

thank you @Staacks for the fix and @kkitayam for reviewing

PS: actually with this current change, I think we could just simply remove the TU_VERIFY() check, but that can be done later.

@hathach hathach merged commit 83cc71f into hathach:master Jan 7, 2023
@Staacks
Copy link
Contributor Author

Staacks commented Jan 9, 2023

Great, thanks! Now I have to figure out why M1/M2 platforms discard half the transmitted data while it is working perfectly on MacBooks with Intel CPUs...

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.

3 participants