First try rusb-async

[a1ien]

rusb-async also published on crates-io https://crates.io/crates/rusb-async

[JJTech0130]

Does this work for isochronous transfers? https://github.com/JJTech0130/easycap-rs/blob/master/src/streaming.rs is my code, but it's just printing 0s

[a1ien]

Hi. Thank you for trying the current api. I haven't tested isochronous endpoints. What operating system are you trying on?

[JJTech0130]

macOS. I’m trying to port https://github.com/JJTech0130/easycap to Rust, it used libusb from Python. I’m fairly new to Rust so it might just me doing something wrong

[a1ien]

Oh it looks like I've implemented the isochronous API incorrectly. I'll try to fix it as soon as I have time. Possibly this week

[a1ien]

I completely forgot that isochronous transfer is different from other. Need more time to think how to implement better api for them. Because for iso we should return list of packet

[JJTech0130]

Wait, so I was trying to just add a debugging print! to transfer_cb:

println!("{}", transfer.iso_packet_desc[0].actual_length);

But then I realized, in the definition for ffi::libusb_transfer:

#[repr(C)]
pub struct libusb_transfer {
    pub dev_handle: *mut libusb_device_handle,
    pub flags: u8,
    pub endpoint: c_uchar,
    pub transfer_type: c_uchar,
    pub timeout: c_uint,
    pub status: c_int,
    pub length: c_int,
    pub actual_length: c_int,
    pub callback: libusb_transfer_cb_fn,
    pub user_data: *mut c_void,
    pub buffer: *mut c_uchar,
    pub num_iso_packets: c_int,
    pub iso_packet_desc: [libusb_iso_packet_descriptor; 0],
}

What's with the 0 length array? The internet seems to say that any values you put into are dropped? What's going on here? Am I missing something?

[JJTech0130]

It turns out that was directly translated from the original C++ header:

#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)
#define ZERO_SIZED_ARRAY        /* [] - valid C99 code */
#else
#define ZERO_SIZED_ARRAY    0   /* [0] - non-standard, but usually working code */
#endif /* __STDC_VERSION__ */

// --snip--

struct libusb_transfer {
    libusb_device_handle *dev_handle;
 
    uint8_t flags;
 
    unsigned char endpoint;
 
    unsigned char type;
 
    unsigned int timeout;
 
    enum libusb_transfer_status status;
 
    int length;
 
    int actual_length;
 
    libusb_transfer_cb_fn callback;
 
    void *user_data;
 
    unsigned char *buffer;
 
    int num_iso_packets;
 
    struct libusb_iso_packet_descriptor iso_packet_desc[ZERO_SIZED_ARRAY];
};

I have no idea what that might mean, though? https://stackoverflow.com/questions/9722632/what-happens-if-i-define-a-0-size-array-in-c-c seems to suggest it's a non-standard, and that sometimes it's where you want to define the size later and allocate using malloc? And someone suggested you could just use it as a pointer?

[JJTech0130]

Looking at the Python implementation, they do:

# Isochronous packet descriptors, for isochronous transfers only.
("iso_packet_desc", (iso_packet_descriptor * 0)),

and then

list_type = libusb_iso_packet_descriptor * transfer.num_iso_packets
return list_type.from_address(addressof(transfer.iso_packet_desc))

I'm starting to think they're just using it as some kind of pointer?

[JJTech0130]

Turns out it's called a "struct hack": to make the last member of the struct have a variable size. Seems very unsafe lol. https://www.geeksforgeeks.org/struct-hack/ Not sure how one would implement it in Rust

[a1ien]

Yes it's C struct hack and it's work in rust for repr C struct. Example bellow

use libusb1_sys as ffi;
use std::ptr::NonNull;

fn main() {
    unsafe {
        let ptr =
            NonNull::new(ffi::libusb_alloc_transfer(5)).expect("Could not allocate transfer!");

        let transfer: *mut ffi::libusb_transfer = ptr.as_ptr();
        (*transfer).num_iso_packets = 5;

        ffi::libusb_set_iso_packet_lengths(ptr.as_ptr(), (200 / 5) as u32);
        for i in 0..(*transfer).num_iso_packets {
            let len = (*transfer)
                .iso_packet_desc
                .get_unchecked_mut(i as usize)
                .length;
            println!("len of #{i} is {len}")
        }
    }
}

[JJTech0130]

This is what I came up with yesterday, forgot to add it here:

    /// Prerequisite: must be an isochronous transfer
    fn iso_packet_descriptors(&self) -> &[ffi::libusb_iso_packet_descriptor] {
        assert!(self.transfer().transfer_type == LIBUSB_TRANSFER_TYPE_ISOCHRONOUS);
        // The packet descriptors are stored using the "C99 struct hack":
        // Basically, there is a zero sized array at the end of the struct
        // and the struct is allocated with extra memory added to the end
        // the size of the extra memory goes to the last element of the struct
        unsafe {
            let addr = addr_of!(self.transfer().iso_packet_desc)
                .cast::<ffi::libusb_iso_packet_descriptor>();
            slice::from_raw_parts(addr, self.transfer().num_iso_packets as usize)
        }
    }

[a1ien]

Looks good. Are you try this with iso? I don't have device with iso endpoint, so I can't test. Also i found another misconception in current API. I try to do some rework on weekend.

[JJTech0130]

Yes, the reason I want to use rusb in the first place is this device called “EasyCap” which allows capturing composite audio + video. Wanted a rust implementation as the python version was too slow. It uses isochronous transfers for streaming video.

[JJTech0130]

I’m wondering how you want to implement this: should isochronous transfers have their own poll function, with a different return type, or just have one function that can return multiple types? I’m leaning towards having multiple functions: that way it can be guaranteed at compile time what type it will return. I’m also wondering if the transfer type should be stored in the struct some how? Or even have multiple structs implementing common traits?

[a1ien]

I am still not sure. Maybe enum or something that contain enum inside. Also pool should process only one endpoint or we should pass transfer into user data.

[JJTech0130]

I'm thinking of restructing it so that the Transfer struct implements common functionality, and there are separate IsochrnonousTransfer, ControlTransfer, and BulkTransfer structs that each have a Transfer struct inside. They will use the standard ::new() constructor.

I'll make a PR soon.

~~The other thing is, it should probably use standard Futures, right?~~ I misunderstood what they are for I think

[JJTech0130]

Also, I think that the Transfer structs should be cleaned up and given safe wrappers... I don't like the whole tie it into a pool thing... imho the user should be free to use Tokio or another library. And it looks like Futures are the right thing to do in this scenario

[a1ien]

imho the user should be free to use Tokio or another library. And it looks like Futures are the right thing to do in this scenario

It's really hard to do. Because libusb it completion based is really hard to do safe wrapper. If you not see this post https://without.boats/blog/io-uring/ I strong recommend read it first.

[JJTech0130]

Well, I'll leave the other part for now, I just want ISO transfers working. Here's what I'm thinking: split the transfers into bulk.rs, isochronous.rs, etc. with this being the contents:

use crate::{error::Result, Transfer};

use rusb::ffi::{self, constants::*};

use std::ptr::NonNull;
use std::sync::atomic::AtomicBool;

pub(crate) struct BulkTransfer {
    transfer: Transfer,
}

impl BulkTransfer {
    // Invariant: Caller must ensure `device` outlives this transfer
    unsafe fn new(
        device_handle: *mut ffi::libusb_device_handle,
        endpoint: u8,
        mut buffer: Vec<u8>,
    ) -> Result<Self> {
        // iso_packets is 0 for non-isochronous transfers
        let ptr = NonNull::new(ffi::libusb_alloc_transfer(0))
            .expect("Could not allocate transfer");

        let user_data = Box::into_raw(Box::new(AtomicBool::new(false))).cast::<libc::c_void>();

        let length = if endpoint & LIBUSB_ENDPOINT_DIR_MASK == LIBUSB_ENDPOINT_OUT {
            // for OUT endpoints: the currently valid data in the buffer
            buffer.len()
        } else {
            // for IN endpoints: the full capacity
            buffer.capacity()
        }
        .try_into()
        .unwrap(); // should not panic as we are expecting the length to be less than 2147483647

        ffi::libusb_fill_bulk_transfer(
            ptr.as_ptr(),
            device_handle,
            endpoint,
            buffer.as_mut_ptr(),
            length,
            Transfer::transfer_cb,
            user_data,
            0,
        );

        Ok(Self { transfer: Transfer { ptr, buffer } })
    }

    fn submit(&mut self) -> Result<()> {
        self.transfer.submit()
    }

    fn cancel(&mut self) {
        self.transfer.cancel();
    }

    fn handle_completed(&mut self) -> Result<Vec<u8>> {
        self.transfer.handle_completed()
    }
}

[JJTech0130]

Looks like the pool would have to be reworked a little: it expects all transfers to be the same. A trait and generics should work though

[JJTech0130]

Oooh, but that causes issues with vector dequeue

[JJTech0130]

Because it could have a different size (as it's a dyn trait), it must be boxed or a reference... can we just Box<> the transfers?

[JJTech0130]

Ok, I put the Transfer in a Box, and I moved the common functionality (submit, cancel, handle_completed) into a trait. The new function is not in the trait, because it takes different arguemnts, and when we're constructing it we know the concrete type anyway.

[JJTech0130]

What's with this:

Box::into_raw(Box::new(AtomicBool::new(false))).cast::<libc::c_void>();

?

[JJTech0130]

Y’know what, I’m going to stop trying to restructure it. I’m realizing that I can’t keep using OOP principles here… not sure what the alternative is though…

[JJTech0130]

So, I'm seeing several things. One of which, is there isn't actually a way to tell what transfer is what, is there? My usecase has an audio stream and a video stream, the audio stream is bulk and the video isochronous. I had a simple workaround where it would simply get the total length of all the packets and dump them into the buffer, thinking I could parse them on the other end. The problem I'm seeing is there is no good way to separate my audio and video responses, is there? Other that just guessing based upon the length of the buffer. And there's no nice "resubmit" like there is for the Python API.

[JJTech0130]

Maybe an enum like you said:

enum TransferType {
    Bulk { endpoint: u8 },
    Control { request_type: u8, request: u8, value: u16, index: u16 },
    Isochronous { endpoint: u8, num_packets: u32 },
}

If we passed that to UserData, it would be available on the other end, and we could keep track of what transfer it was. Would also allow for a quick re-submit. Not sure if the buffer should be part of the enum or just something in the actual transfer. You would then pass this "metadata" into the actual transfer.

[JJTech0130]

Also oops totally forgot about implementing Interrupt transfers...

[brandonros]

@a1ien can you make rusb_async::Error public so we can match on PollTimeout errors please?

error[E0603]: module `error` is private
   --> src/main.rs:181:37
    |
181 |                         rusb_async::error::Error::PollTimeout => {
    |                                     ^^^^^ private module
    |

[a1ien]

Hi. Sure, but i want little rewrite this approach. I found a few more omissions. I try to find time and make new release at the end of October.