yury / cidre Goto Github PK

Is there a way to have a sync version or add Send to get sharable content? Because I'm getting error of cannot send !Send struct across await points.
https://github.com/yury/cidre/blob/main/cidre/src/sc/shareable_content.rs#L113

Other impls I've seen: https://github.com/svtlabs/screencapturekit-rs/blob/main/screencapturekit-sys/src/shareable_content.rs#L175

From a quick skim, I found the following important safety mistakes, that leads to unsound code (unsoundness = a safe abstract which a downstream user can use to violate memory safety without writing unsafe themselves).

Note that I completely understand the reasoning behind these mistakes, most have only been fixed in objc2 very recently!

• arc::Retain<T> allows access to &mut T (through DerefMut), which is not safe for almost all classes.

  use cidre::ns;
  
  let mut s1 = ns::String::with_str("already lowercase");
  let mut s2 = s1.retained();
  let s1: &mut ns::String = &mut *s1;
  let s2: &mut ns::String = &mut *s2;
  // The mutable references s1 and s2 now alias

• Autoreleased references are not bound to a pool, meaning that they can be accessed after they've been reclaimed by the pool.

  use cidre::{ns, objc};
  
  let s = ns::String::with_str("My String");
  let lowercased = objc::autoreleasepool(|| {
      s.lowercased_ar()
      // The newly created string is released and deallocated here
  });
  // But we can still use it here
  println!("{lowercased}");

• Message sending to methods in the new family leaks, since new returns an object with +1 retain count.

  use cidre::mtl;
  let device = mtl::Device::default().unwrap();
  
  // Created with +1 retain count, and retained, so it ends up at +2
  let cmd_queue = device.new_cmd_queue();
  // Retain count decremented only once
  drop(cmd_queue);
  // The CmdQueue is leaked

https://evanw.github.io/glfx.js/
https://medium.com/@evanwallace/easy-scalable-text-rendering-on-the-gpu-c3f4d782c5ac
https://github.com/evanw/theta

Hey @yury, we talked a bit about about this over email, but I wanted to elaborate in full.

I think it would be beneficial for our two projects to collaborate, since we're both defining API(s) over Apple's frameworks - especially our abstractions over the Objective-C runtime would benefit from collaboration.

Concretely, I think a shared codebase for this is the way to go.
I believe the best way forward here would be for cidre to build upon objc2, since objc2 was built with the idea of it being a library that others can use like this - but if you think it would make more sense to make some kind of objc-core crate, I would be fine with that as well. In general, just sharing ideas and discussing things should also provide a great benefit to both of us, so please treat this proposal of cidre moving to objc2 as just that; a proposal ;).

Looking at your stated goals, I think the bits about performance is fully compatible with mine, I very much want to make it possible to have fast, zero-cost interaction with Objective-C! Your other goals, i.e. Rusty API / ML friendly, can still be pursued by this crate while using objc2.

Although objc2 tries to support older platforms, your explicit goal of not doing so is not incompatible with it; I would be completely on-board with adding a feature flag to enable newer features that would yield higher performance (in fact, I am working on functionality to do this based on the deployment target, so the user wouldn't even have to do anything).

With that out of the way, let me try to outline the pros and cons of cidre moving to objc2:

Pros

• Vastly improved safety/soundness. See #3 for a few of the cases that would be fixed by using objc2.
• Interoperability with the rest of the ecosystem (provided people decide on objc2 being "the ecosystem", of course).
• A shared place to add performance improvements (so if I find something that can be improved, cidre would benefit as well).
• Less code for you to maintain.
• More eyes on and real-world usage of the shared code.

Cons

• Since Allocated<T> is a struct in a different crate, you can't implement init methods for this the same way you're currently doing it. Specifically, you would either have to provide new methods that does the allocation internally, or live with having to do Data::init_with_bytes(Data::alloc(), bytes) in the interrim.

  Note that arbitary self types are somewhat nearing stabilization, which would solve this issue completely.

• objc2::rc::Id (the equivalent to cidre::arc::Retained) does not allow overriding the retain/release implementations, so CoreFoundation may be a bit harder for you to support.

  I am still not sure of the best approach here, so very willing to discuss this part!

• The code lives somewhere that you don't control, which will make it harder for you to make changes.

What do I gain?

I'm trying to win you over to using objc2 because I believe your input, as well as performance improvements you find, would be valuable to the project.

After all, the more use-cases objc2 can satisfy, the more users it has, and the better off the project is!

Implicit in the above is that I'd be willing to shape objc2 to fit your goals, if needed!

I hope you don't take offense to this proposal, again, if you disagree with this, I would still like to collaborate!

Feel free to contact me on email or on Element (@madsmtm:matrix.org) if you want to discuss things in a private medium/more synchronously.

I see that you're emitting a call to a objc_msgSend function, but you somehow move the selector into the link name. How does that work? I assume it's dyld magic, but do you have some documentation on it, and some information on since when it has been supported?

I ran cargo update to use the latest commit. Suddenly this line of code caused a crash:

cidre::sc::ShareableContent::current().await;

without any stack trace but this error from my allocator (mimalloc):

Memory released without being allocated first.

I downgraded to this commit: 94c379f
and issue was resolved. So I think the commit causing this is: 2783e7b

Thanks for the awesome library!
I'm getting this error in my Github Actions build:

error: invalid register `x0`: unknown register
  --> /Users/runner/.cargo/git/checkouts/cidre-e660120dbd7e907a/2ecba7a/cidre/src/objc.rs:86:13
   |
86 |             lateout("x0") result,

Any idea how can I fix this/patch this?

I've added this type for encoder specifications for our app:

pub mod specs {
  use cidre::cf;

  #[link(name = "VideoToolbox", kind = "framework")]
  extern "C" {
    static kVTVideoEncoderSpecification_EnableLowLatencyRateControl: &'static cf::String;
  }

  #[doc(alias = "kVTVideoEncoderSpecification_EnableLowLatencyRateControl")]
  #[inline]
  pub fn enable_low_latency_rate_control() -> &'static cf::String {
    unsafe { kVTVideoEncoderSpecification_EnableLowLatencyRateControl }
  }
}

Where do you think is appropriate to put this in cidre? I'll make a PR.

(fyi low latency rate control is awesome: https://developer.apple.com/videos/play/wwdc2021/10158/#)