openfx icon indicating copy to clipboard operation
openfx copied to clipboard

New C++ helpers

Open garyo opened this issue 11 months ago • 5 comments

Open Effects Proposal for Standard Change

Please read the contribution guidelines first.

Standard Change Workflow

  • [x] Create proposal as issue (you're doing this now!)
  • [x] Tag this issue with standard change tag
  • [ ] Identify subcommittee: at least one plug-in vendor, and at least one host
  • [ ] Discuss the idea in this issue
  • [ ] Write new or updated code and doc
  • [ ] Publish updates as a pull request (ideally on a feature/PROPOSAL-NAME branch)
    • [ ] Make sure that PR references this issue number to keep them in sync
    • [ ] Discuss and review code in the PR
    • [ ] Meet all requirements below for accepting PR
  • [ ] When subcommittee signs off and other members don't have any further review comments, maintainer merges PR to master which closes PR and issue

Requirements for accepting a standard change:

  • [ ] Header files updated
  • [ ] Documentation updated
  • [ ] Release notes added
  • [ ] Compatibility review completed
  • [ ] Working code demonstrated with at least one host and one plugin
  • [ ] At least two members sign off
  • [ ] No further changes requested from membership

Summary

I’m beginning to work on a “modern C++” openfx-cpp support lib that will be more maintainable, type-safe and easy to use, as well as reflecting the C API more directly. I expect to contribute this to the main repo at some point. A couple of questions for you:

  • Would you use such a thing when creating new plugins?
  • Should I aim for C++20 (better, more compile-time type safety etc.) or C++17 (in more common use today)?
  • Are there particular pain points you have with implementing plugins using the C API? I’m thinking initially about props, params, clips and images.

Motivation

The current C++ support lib is very out of date, and hasn't been updated in a long time. It also doesn't use any modern C++17 or later elements, so it is clunky to use. It also works only as a complete framework, not as an incremental add-on to the C API. This proposal is for a more minimal set of C++ classes for clips, images, properties, and parameters, to simplify access and ensure type-safety. It uses the new type info for all built-in properties and property sets to enable simple type-safe property access.

Problem

The goal is to simplify development of new plugins for new users of the API, while staying true to the original design goals of simplicity, compatibility and extensibility.

Impact

This is entirely plugin side, and has no impact on the C API at all.

Documentation Impact

The new helpers will need documentation as well as sample effects.

Stakeholders

This is intended for plugin creators.

Discussion

Initial implementation is taking place in the https://github.com/AcademySoftwareFoundation/openfx/tree/props-metadata branch, see #166 .

garyo avatar Mar 04 '25 15:03 garyo

I am not totally following latest standard debates but reading a bit yesterday, just some observations, not a particular PR.

Seems the C++ discussion, is their idea is to make a subset of C++ called safe C++ and make the rest of C++23 (or C++26) optional, opt-in. You might have without paying attention heard that all US government agencies have put out recommendations to try to avoid C/C++ because of increased security issue - hacking, e.g.: https://www.tomshardware.com/software/security-software/white-house-urges-developers-to-avoid-c-and-c-use-memory-safe-programming-languages

Unclear all that applies to folks like us who were trained on 4 Mg of RAM ( this fast read basically sums up to a list of supposively bad as in e.g. no manual allocation: malloc, new, free, delete - no pointer arithmetics, no varargs, no void, I see new threading model in 2023, no reinterpret cast to downcast,...) - and being c API based might make this something that cannot be follow pedantically. I imagine some of this comes from people learning to program using pointer free languages like Java, and piled-on by rustafarians...

I was reading for example that Google pulled out support for jpeg-xl because it was too large of a C/C++ surface to protect for browsers...

Taking C++ criticism literally: Resource management: The main item is memory management according to C++ standards leaders.

What could this mean for us? Again just thinking loud... We don't leak memory here :) Here we have a DEBUGMEMORY mode here that does book-keeping on every allocation via the memory suite for sanity purpose that can be armed prior to release. We allocate large memory pools not to create fragmentation... I also a while ago put in next for GPU FR, handshake with host type thing that do GPU memory management. One thing I see too is some host (and|or IO libraries when dealing with long-gop) use VM - short form: if we use a lot of intermediate GPU memory on high resolution video or low VRAM cards, in some apps it's easy to go into VM - with current drivers we don't crash as driver swap GPU memory but it becomes incredibly slow renders.

For pointer arithmetic being bad - not quite clear - what else can one do? use span with per scan line indexing? std:span introduced in C++20 is just a pointer + size no? How does that help, we have rowbytes and type and number of channels - this could just be a macro no?

For varargs I remember Gary initially was not a fan of example using VarArgs... what else could that be at C++ wrapper level?

It looks like since c++17 it is recommanding to use std::any instead of void * as we do?

Other side topic is some companies do already have guidelines for what can be done in C++, one my CTO friend calls it C+ as they had issue with new kids showing off their C++ skills and producing code no one understood. Even the language author says the standard has gone overboard, he does not even know a lot of C++ anymore himself. Debugging someone else code is real important, to keep the stack trace real clear and navigable in an IDE.Anything that create obfuscation at that level is more harmful then useful. We also here put everything in try - catch and don't allow to return in a middle - this prevents many multi-developer touch the code issues. This makes debugging real easy on code you never saw before. Printf Debug folks have issue with that. There are cases with log to file makes sense: finding where a time critical race condition occurs and things like that where errors won't occur in debug.

revisionfx avatar Mar 24 '25 19:03 revisionfx

The OpenFX API must remain "pure" C to avoid C++ ABI incompatibilities between host and plugin. Either side is free to layer C++ on top of that, as we know. Furthermore, any language which implements the pure C ABI can be used for a host or plugin: i.e. Rust or Go or even Python with a foreign function interface. This issue is about a modern C++ API.

I don't think OpenFX needs to do anything regarding the Safe C++ discussions underway now, though if we do promote a particular C++ helper library, as I'm suggesting here, it is important that that library be focused on safety and security, with as much as possible checked at compile time (constexpr etc.) and we provide unit tests to ensure the library functions as intended.

I don't expect the C++ API I'm proposing to do much if any memory allocation. There may be some initializations at startup time but I am trying to keep as much const/constexpr as possible. It does use std::string (or std::string_view for non-owning strings) everywhere. (C++20 introduces nice helpers for strings which would be useful.)

As I said at the beginning, in my C++ helpers I will use "modern" C++ techniques. I would love to use std::span but that is only in C++20 -- perhaps my greatest reason for preferring that. Other folks should weigh in. Pointer arithmetic should of course be avoided in the helpers. Of course a given algorithm will almost certainly use pointer arithmetic during its render. That's not a concern for the API or the C++ layer. Yes, a span is just pointer + size, which allows for both compile-time and runtime checks (when enabled).

C-style varargs should absolutely be avoided! Modern C++ has better type-safe ways of doing variable arg lists (variadic templates, arg packs, std::forward, etc.) Check out std::format for a good example -- a 100% type-safe (compile-time checked) formatting lib.

std::any is a fallback that can be used if needed, but my goal is to have type-safe accessors throughout, avoiding casts except when absolutely needed.

I am planning to use exceptions for error handling, rather than some of the alternatives often discussed (std::variant, std::optional etc.) to keep the "happy path" clean.

As usual for a modern C++ library, the implementation will involve complicated templates and advanced techniques, but the goal is to provide a reasonably small, type-safe clean API with reasonable error messages to catch many errors at compile time.

I am including a very small pluggable logging subsystem and a set of standardized exceptions, as you suggest, Pierre. I hope that addresses your concerns!

garyo avatar Mar 25 '25 14:03 garyo

Is you concern about usingstd::optional or std::expected over exceptions purely about a clean 'happy path'?

bruno-j-nicoletti avatar Mar 25 '25 18:03 bruno-j-nicoletti

@bruno-j-nicoletti Never heard of std:expected :) - Looks like it's C++23 - @garyo did a first pass for sanitizing property testing (some properties in process have even been renamed) and a base for meta-data... and looking at doing a new generation of the C++ wrapper - we are not clear at what C++ version we should floor this.

revisionfx avatar Mar 25 '25 18:03 revisionfx

Hi @bruno-j-nicoletti ! Yes, I'd like users to be able to do things like this:

      paramProps.set<PropId::OfxParamPropDefault, double>(default_value)
          .set<PropId::OfxParamPropIncrement>(increment)
          .set<PropId::OfxParamPropMin>(min_value)
          .set<PropId::OfxParamPropMax>(max_value);

(just as an example) And I'm pretty sure C++23 is out for this project, sadly -- tons of great stuff in there. C++20 support is common enough that I'm hoping by the time this reaches maturity people (=users of this C++ wrapper) will be OK with it.

garyo avatar Mar 26 '25 12:03 garyo