Ruslan-B
decorating a few structures...
-
**I'm submitting a ... **
- [ ] bug report
- [x] feature request
- [ ] support request or question => Please do not submit support request or questions here, see note at the top of this template.
-
Do you want to request a feature or report a bug?
Following the addition of partials, I've been experimenting with what can be done, I forked the repo and I added these two decorations:
Let me know your thoughts, and if you would accept such a PR
About AVBufferRef, it's just an example, I think exposing all pointer/size field pairs as Span<Byte> properties would greatly help making the library much safer to use.
In general I like the idea. However, when I see property which going to return something different every time you read it - my eyes are getting wet 🤣. This must be methods not properties for sure.
In the past I saw few projects started on top of FFmpeg.AutoGen with the goal to build safe abstraction - the only one thriving now is FFMediaToolkit. So it would be nice to get feedback from most active users/consumers.
I don't mind making these properties methods, although I don't think theres any overhead returning Span<T> objects since spans are treated essentially as pointers by the runtime.
I don't think it's wise to write a full, rich wrapper with complex methods.... it's more about exposing the same data types the structures already expose in a more C# way.... like pointers to arrays returned as spans and so on.
I also don't pretend to write a full PR for all data types, just for a few ones I use... it will also give everybody time to see if it's a good idea or not
I don't mind making these properties methods, although I don't think theres any overhead returning
Span<T>objects since spans are treated essentially as pointers by the runtime.
No, it isn't just a pointer - Span is a ref value type and besides pointer it has a length as well. In general it is commonly accepted it is a bad practice: https://stackoverflow.com/questions/2101646/is-object-creation-in-getters-bad-practice
I don't think it's wise to write a full, rich wrapper with complex methods.... it's more about exposing the same data types the structures already expose in a more C# way.... like pointers to arrays returned as spans and so on.
I also don't pretend to write a full PR for all data types, just for a few ones I use... it will also give everybody time to see if it's a good idea or not
I also don't like the approach - let's throw something in and see what happens next. Besides, what is good for you doesn't means good for others. Not speaking it potentially will look as work half done. I'll think what to do with this.
Here's my 2cent, feel free to ignore. Basically the same as vpenades, just in many more words...
A library should be consistent. If some goodies are only available for 5% of the API and unavailable for the other 95%, than that's not good because it confuses the library user ("why is this not available here? Maybe I should not use it and am doing something wrong?"). The consistency goal should not prevent one from starting with just a few structs, but once accepted the long term goal should be to have a consistent level accross a majority of the use cases of the API.
FFmpeg.Autogen's goal is to make FFmpeg accessible in a "C# way". No less, no more. That should be the guideline on deciding what stuff to include and what to keep out, even when it's useful. The AvBufferRef example from vpenades is imho clearly on the "make it the C# way" side of things.
I liked the way the (now defunct) SharpDX library did things for DirectX COM calls: convert stuff consistenly to a C# style signature while not changing the behaviour of the underlying COM. For example ID3D11Texture::GetDesc(struct Description) becomes a property Texture.Description, because that's how one expect an object to behave in C#. Also Dispose was introduced accross the board, because that's what C# people know. And finally methods where the underlying library returns an error code throw exceptions, because again that's the C# style and C# people tend to forget to check result codes for every stupid simple library call.
On the other hand, the "throw exception" example illustrates what a slippery slope this can become: there are library calls where a negative result is perfectly valid. For FFmpeg think of AVERROR(EAGAIN) on receive_packet. So the wrapper library must manually adjust for these cases because having one exception per decoded frame is very ugly. At this point one leaves the "automatic conversion via CppCsWrapper" land and must deal with the logic of FFmpeg.
Having said that, here's my personal top 4 of stuff I'd love to see in FFmpeg.AutoGen. Just as an idea.
- ToString() or DebuggerDisplay for ffmpeg data structures ("record struct" might help here. The record thing seems arcane at first, but basically it's just the compiler generating some additional helpful stuff like ToString and Equals for you)
- Automatically throw exceptions on errors. On might need to manually create and maintain a list of ffmpeg methods where negative values are not errors, but I think it's worth the effort...
- Property Access to things that are logically properties. Like AVCodec.Name and AvCodec.LongName string properties in addition to name and long_name byte arrays. Span access to pointers also falls into this category imho
- IDisposable where appropriate.
P.S. Small comment on the span/property discussion: Span<T> is a struct that's allocated on the stack and not the managed heap (as per docs). So imho it's low level overhead and fine for a property. It's not "create new object for each property get access" type of badness. But how bad that overhead is depends on the typical level of abstraction a developer is working on. For a high level developer Person.FullName => $"{FirstName} {LastName}"; is perfectly accetable while for a low level game developer this is excessive "create new class on every property get access" overhead
regarding pointers, I know Spans are ref structs, but they're optimized to pointers when they're compiled... Spans are a very special case that is treated differently than other structs by the compiler.
and see what happens next
Ah, sorry for the misunderstanding. I wrote these two decorations as showcase, for you and others to see what you like and what you dislike... that's why I didn't open a PR yet.
After all, many decorations can be achieved by extensions in a third party libraries. The only two things that cannot be added as an extension is the DebuggerDisplay attribute and the .Dispose, these must be in the core structure, so if you want to keep manual code to a minimum, at least the partials should handle these two.
I don't have a knowledge so deep to add .Dispose to all methods that require it... but at least I could add a PR with the DebuggerDisplay attribute.
Also, I didn't consider what @digivod has suggested about Dispose. If there's some references that need to be freed, it would be nice to have a Dispose in these structures, so we can use using patterns, which are much nicer than bloated try catch finally blocks
Let park this topic, as from my point of view the goal is to provide library which is close to C(lang) as much as could in C# - this simplifies consumption of the API and examples - as it mostly true that you can correctly port any of the stack based allocation sample from ffmpeg. The firm things from my side - if I like to extend - we can use extensions and can decorate the main library. For second I cannot commit to improve API on a scale. It isn't easy and I don't have a feeling we can pull it now - we just in the moment when community started to form. I can do a lot, however, we have to do this over a time - so I am thinking about other repo which will be based on auto generated but will be manually and carefully crafted.
Agreed. When somebdody wants to extend stuff and make the extensions available for everybody, imho the examples sections are a good place to do so. Maybe via adding an "Examples.Common" project that holds extensions that are generally helpful. Having good examples helps people getting started with a library.
I'll try to come up with something when I find the time.
The "Disposable" stuff is most likely very hard to do and so out of scope. I just wished for it many times, because I tend to get lost in FFmpegs "what do I have to _close, _free or _unref" zoo...