dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
157 stars 45 forks source link

Have trouble when covert Array<UnsignedChar> to Dart String. #1601

Closed yanshouwang closed 2 months ago

yanshouwang commented 2 months ago

Recently I'm working on using ffigen to call v4l2 on linux, there is a v4l2_capability Struct which is generated like this:

/// struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
///
/// @driver:       name of the driver module (e.g. "bttv")
/// @card:     name of the card (e.g. "Hauppauge WinTV")
/// @bus_info:     name of the bus (e.g. "PCI:" + pci_name(pci_dev) )
/// @version:      KERNEL_VERSION
/// @capabilities: capabilities of the physical device as a whole
/// @device_caps:  capabilities accessed via this particular device (node)
/// @reserved:     reserved fields for future extensions
final class v4l2_capability extends ffi.Struct {
  @ffi.Array.multi([16])
  external ffi.Array<__u8> driver;

  @ffi.Array.multi([32])
  external ffi.Array<__u8> card;

  @ffi.Array.multi([32])
  external ffi.Array<__u8> bus_info;

  @__u32()
  external int version;

  @__u32()
  external int capabilities;

  @__u32()
  external int device_caps;

  @ffi.Array.multi([3])
  external ffi.Array<__u32> reserved;
}

The corresponding c struct is:

/**
  * struct v4l2_capability - Describes V4L2 device caps returned by [VIDIOC_QUERYCAP](https://docs.kernel.org/userspace-api/media/v4l/vidioc-querycap.html#vidioc-querycap)
  *
  * @driver:       name of the driver module (e.g. "bttv")
  * @card:         name of the card (e.g. "Hauppauge WinTV")
  * @bus_info:     name of the bus (e.g. "PCI:" + pci_name(pci_dev) )
  * @version:      KERNEL_VERSION
  * @capabilities: capabilities of the physical device as a whole
  * @device_caps:  capabilities accessed via this particular device (node)
  * @reserved:     reserved fields for future extensions
  */
struct v4l2_capability {
        __u8    driver[16];
        __u8    card[32];
        __u8    bus_info[32];
        __u32   version;
        __u32   capabilities;
        __u32   device_caps;
        __u32   reserved[3];
};

And I convert the Array to String as followed:

extension UnsignedCharArrayX on ffi.Array<ffi.UnsignedChar> {
  String get dartValue {
    final charCodes = <int>[];
    var i = 0;
    while (true) {
      final charCode = this[i];
      if (charCode == 0) {
        break;
      }
      charCodes.add(charCode);
      i++;
    }
    return String.fromCharCodes(charCodes);
  }
}

When I run the app, the card and bus_info are correct, but the driver field is always incorrect and changed every time I run the code.

Here is the logs

flutter: driver Rå‘7~x, card USB Camera, bus usb-0000:00:14.0-6
flutter: driver '›‡, card USB Camera, bus usb-0000:00:14.0-6
flutter: driver H`—7~x, card USB Camera, bus usb-0000:00:14.0-6
liamappelbe commented 2 months ago

How are you getting an instance of that struct? Assuming you're getting it from some function like foo(v4l2_capability* out_capability), can you write a wrapper around that function in C, and print the values of those strings after foo returns but before control passes back to Dart? If the strings are valid at that point, it'll tell us if this is a problem with the C API or with Dart FFI.

yanshouwang commented 2 months ago

@liamappelbe I have found out the reason why the string is wrong.

The string is correct when I use it inside the using block, but it's wrong when I use it outside using block.

I think I can‘t use the ref struct after the pointer is freed?

final cap = ffi.using((arena) {
      final capPtr = arena<ffi.v4l2_capability>();
      final error = ffi.libV4L2.ioctl(device.fd, ffi.VIDIOC_QUERYCAP, capPtr);
      if (error == -1) {
        throw V4L2Error('ioctl failed, $error.');
      }
      // Here the driver is correct.
      print(capPtr.ref.driver.dartValue);
      return capPtr.ref;
    });
    // Here the driver is wrong.
    print(cap.driver.dartValue);
liamappelbe commented 2 months ago

I think I can‘t use the ref struct after the pointer is freed?

Yeah, the struct won't be valid anymore after the backing storage is deleted. I'm doing something similar to return structs by value in this PR. You could try switching to this pattern instead (note how _ptr and _data are used to create the returned struct):

/// twiddleVec4Components:
Vec4 twiddleVec4Components_(Vec4 v) {
  final _ptr = pkg_ffi.calloc<Vec4>();
  final _data = _ptr
      .cast<ffi.Uint8>()
      .asTypedList(ffi.sizeOf<Vec4>(), finalizer: pkg_ffi.calloc.nativeFree);
  objc.useMsgSendVariants
      ? _objc_msgSend_5Stret(
          _ptr, this.ref.pointer, _sel_twiddleVec4Components_, v)
      : _ptr.ref =
          _objc_msgSend_5(this.ref.pointer, _sel_twiddleVec4Components_, v);
  return ffi.Struct.create<Vec4>(_data);
}
liamappelbe commented 2 months ago

@dcharkes Would it make sense to add a util in package:ffi that does this allocation dance? I tried writing a util to use in the ffigen bindings, but I think it'll need some CFE magic. In that snippet, if you try to make that allocation generic (ie swap out Vec4 with T), then you get a bunch of compile errors because several of those generic functions need to have the concrete type directly between the <>.

dcharkes commented 2 months ago

I think I can‘t use the ref struct after the pointer is freed?

Yeah, the struct won't be valid anymore after the backing storage is deleted. I'm doing something similar to return structs by value in this PR. You could try switching to this pattern instead (note how _ptr and _data are used to create the returned struct):

/// twiddleVec4Components:
Vec4 twiddleVec4Components_(Vec4 v) {
  final _ptr = pkg_ffi.calloc<Vec4>();
  final _data = _ptr
      .cast<ffi.Uint8>()
      .asTypedList(ffi.sizeOf<Vec4>(), finalizer: pkg_ffi.calloc.nativeFree);
  objc.useMsgSendVariants
      ? _objc_msgSend_5Stret(
          _ptr, this.ref.pointer, _sel_twiddleVec4Components_, v)
      : _ptr.ref =
          _objc_msgSend_5(this.ref.pointer, _sel_twiddleVec4Components_, v);
  return ffi.Struct.create<Vec4>(_data);
}

Offtopic: It would be better to move final _data = ... after the objc.useMsgSendVariants.

@dcharkes Would it make sense to add a util in package:ffi that does this allocation dance? I tried writing a util to use in the ffigen bindings, but I think it'll need some CFE magic. In that snippet, if you try to make that allocation generic (ie swap out Vec4 with T), then you get a bunch of compile errors because several of those generic functions need to have the concrete type directly between the <>.

So you mean a util in dart:ffi, rather than package:ffi? We don't have CFE magic for anything in package:ffi.

@liamappelbe I have found out the reason why the string is wrong.

The string is correct when I use it inside the using block, but it's wrong when I use it outside using block.

I think I can‘t use the ref struct after the pointer is freed?

final cap = ffi.using((arena) {
      final capPtr = arena<ffi.v4l2_capability>();
      final error = ffi.libV4L2.ioctl(device.fd, ffi.VIDIOC_QUERYCAP, capPtr);
      if (error == -1) {
        throw V4L2Error('ioctl failed, $error.');
      }
      // Here the driver is correct.
      print(capPtr.ref.driver.dartValue);
      return capPtr.ref;
    });
    // Here the driver is wrong.
    print(cap.driver.dartValue);

You can't use arena and then return memory allocated in the arena, it is freed. If you want to create a struct which is freed on finalizer, indeed use the pattern @liamappelbe descibed. calloc + asTypedList (with finalizer) and Struct.create.

Maybe we could add a pattern that does the .refWithFinalizer in dart:ffi. (And then have plenty of documentation that one should not use the original pointer anymore.) Let me file an issue.

yanshouwang commented 2 months ago

@dcharkes @liamappelbe I kept the struct pointer and use Finalizer and Finalizable to keep the memory safe like this.

final finalizer = ffi.NativeFinalizer(ffi.malloc.nativeFree);

/* V4L2Format */
abstract base class V4L2FormatImpl implements V4L2Format {
  V4L2FormatImpl();
  factory V4L2FormatImpl.managed() => _ManagedV4L2FormatImpl();

  ffi.v4l2_format get ref;

  V4L2BufType get type => ref.type.toDartBufType();
  set type(V4L2BufType value) => ref.type = value.value;

  @override
  V4L2PixFormat get pix => V4L2PixFormatImpl.unmanaged(ref.fmt.pix);
  @override
  set pix(V4L2PixFormat value) {
    if (value is! V4L2PixFormatImpl) {
      throw TypeError();
    }
    ref.fmt.pix = value.ref;
  }
}

final class _ManagedV4L2FormatImpl extends V4L2FormatImpl
    implements ffi.Finalizable {
  final ffi.Pointer<ffi.v4l2_format> ptr;

  _ManagedV4L2FormatImpl() : ptr = ffi.malloc() {
    finalizer.attach(
      this,
      ptr.cast(),
    );
  }

  @override
  ffi.v4l2_format get ref => ptr.ref;
}

/* V4L2PixFormat */
abstract base class V4L2PixFormatImpl implements V4L2PixFormat {
  V4L2PixFormatImpl();
  factory V4L2PixFormatImpl.unmanaged(ffi.v4l2_pix_format ref) =>
      _UnmanagedV4L2PixFormatImpl(ref);
  factory V4L2PixFormatImpl.managed() => _ManagedV4L2PixFormatImpl();

  ffi.v4l2_pix_format get ref;

  @override
  int get width => ref.width;
  @override
  set width(int value) => ref.width = value;

  @override
  int get height => ref.height;
  @override
  set height(int value) => ref.height = value;

  @override
  V4L2PixFmt get pixelformat => ref.pixelformat.toDartPixFmt();
  @override
  set pixelformat(V4L2PixFmt value) => ref.pixelformat = value.value;

  @override
  V4L2Field get field => ref.field.toDartField();
  @override
  set field(V4L2Field value) => ref.field = value.value;
}

final class _UnmanagedV4L2PixFormatImpl extends V4L2PixFormatImpl {
  @override
  final ffi.v4l2_pix_format ref;

  _UnmanagedV4L2PixFormatImpl(this.ref);
}

final class _ManagedV4L2PixFormatImpl extends V4L2PixFormatImpl
    implements ffi.Finalizable {
  final ffi.Pointer<ffi.v4l2_pix_format> ptr;

  _ManagedV4L2PixFormatImpl() : ptr = ffi.malloc() {
    finalizer.attach(
      this,
      ptr.cast(),
    );
  }

  @override
  ffi.v4l2_pix_format get ref => ptr.ref;
}

And use it like this:

  @override
  V4L2Format gFmt(int fd) {
    // TODO: add more types.
    final fmt = _ManagedV4L2FormatImpl()..type = V4L2BufType.videoCapture;
    final err = ffi.libV4L2.ioctlV4l2_formatPtr(fd, ffi.VIDIOC_G_FMT, fmt.ptr);
    if (err != 0) {
      throw V4L2Error('ioctl `VIDIOC_G_FMT` failed, $err.');
    }
    return fmt;
  }

I think the native memory can be freed automatically when the dart instance finalized, am I right?