On Mon, 26 Jan 2015 10:42:27 +0000 Robin Stevens <[email protected]> wrote:
> On Sat, Jan 24, 2015 at 11:14 PM, wm4 <[email protected]> wrote: > > On Fri, 23 Jan 2015 20:26:35 +0000 > > Robin Stevens <[email protected]> wrote: > > > > > >> I don't think that level of discoverability is true of the C > >> structs-and-functions API that the ffmpeg libraries currently use. > > > > Idiotic prejudices against C APIs are not helpful here. FFmpeg is > > mostly not a good C API and could be better, simple as that. > > > > I have no such prejudice. The level of discoverability of the C > structs-and-functions API that the *ffmpeg* libraries currently use is > not great. > > It's entirely possible to make discoverable APIs in C; just that I > don't think ffmpeg has one. OK, it's just a bit hard to make APIs in C without structs and functions... > > > > So I wonder what your .net wrapper is supposed to help here. It > > basically turns all public fields into trivial properties that get or > > set the field. What's the use? > > > > That's a fair criticism of much of the wrapper. The use is to expose > the FFmpeg api to C#, the getters and setters help with that. (The > wrapper is in managed C++/CLI, so the interop is taken care of for > me.) > > There is some value in the methods (instance or static) on the types, > which makes for greater discoverability, but doesn't really help you > understand when and where a particular type is meant to be used. Somewhat true, there are way too many API things without context. Reading the headers is not fun at all. > > > Anyway, yes, that's one of the problem with this "let's dump everything > > into a big struct" API. A big class with hundreds of getters and > > setters is better? > > > > It can be, yes. Take the AVFrame struct, and look at the sample_rate > member. Is this a read-only property? Can it be written to? Does that > operation make even make sense? > > Perhaps I want to change the sample rate of a frame. From this API I > might think I just need to set a value on that variable and the > library will magically change the sample rate for me. > > The ffmpeg API does NOT tell me. The only way to find out is to dive > in to the code. That is a deficiency of the interface which should > specify such things. (And my preference is for language support so > that the specification is enforceable by the compiler, but that's just > my preference.) Well, it's kind of obvious (or is it?) that the AVFrame members describe the format of the data it carries. Just setting the sample_rate field can logically not make FFmpeg convert the format. In a way, a setter would make things worse. (But I acknowledge your points.) > My wrapper only has a getter for that property, so it's a syntax error > to attempt to set it. It's clear to the user that they cannot change > the sample rate by setting that property, and need to go and look > elsewhere. You can change it, for example when you initialize the AVFrame and fill it with your own data. Ideally I would say AVFrame should be opaque. There should be a separate struct that describes formats. You could freely modify the format struct, and pass it to a function that creates an AVFrame. This way AVFrame could be fully immutable once created. Trying to push such an API change would probably fail, though. > > C cannot (AFAIK) specify in the language whether a variable is > publicly read-only or writeable, so static checking is not available. > The solution to this, as far as the FFmpeg API is concerned, appears > to be to document in the comment whether the field is writeable. That is not really the problem. The problem is that many fields in FFmpeg structs are read-only, write-only, or both depending on context. Sometimes fields are declared off-limits, but can be legally accessed by AVOption. This is quite hysteric. I also despise structs like AVCodecContext, which have literally dozens of fields, most with questionable value. > That's perfectly reasonable, although not consistently done. Easy to > fix though, if you have the knowledge. > > > Another small win: basic class hierarchy. > > The AVFrame structure encapsulates two very different types of frames: > audio and video; and has a bunch of variables that apply to one or the > other, but not both (and some variables that *do* apply to both). My > wrapper splits AVFrame into a base class, an AVVideoFrame and an > AVAudioFrame. You can achieve the same thing with aggregation. > > This could presumably be done in the FFmpeg API but the devs have > chosen to spend their time elsewhere. > > I'll admit that this is a a tiny, marginal win (at best) over ffmpeg, > given the comment SAYS audio sample rate. Though it *is* a small win, > and lots of those add up. Except, in case of the sample_rate field, you can't make full use of the API anymore. And that's just my point: a wrapper which tries to paint over the issues in the "original" will just suffer from the API mismatches. > > None of the points I'm making amount to much on their own, but they > are I think useful illustrations of some of the problems I've had with > the FFmpeg API. > > Improving and documenting the real API is the best option, but has no > realisitic prospect of success. I someone like me came along and > started suggesting (breaking) changes to the existing API (like > AVFrame, AVAudioFrame, AVVideoFrame), I would (rightly) be shot down > in flames. Yes, making such radical API changes is hard and will take a long time. Maybe it would have a chance of success if it's well planned and agreed upon in advance. FFmpeg had other semi-radical API changes in the past. _______________________________________________ Libav-user mailing list [email protected] http://ffmpeg.org/mailman/listinfo/libav-user
