| Message ID | 20161218122221.23688-1-george@nsup.org |
|---|---|
| State | Superseded |
| Headers | show |
On 12/18/2016 9:22 AM, Nicolas George wrote: > av_buffersink_get_frame_rate() did already exist; its argument becomes const. > > TODO minor version bump What's the purpose of adding these accessors? The only reason we have done this before was because of the now dropped libav ABI compatibility, since we had to add our own new fields below theirs which meant there was no guarantee of a fixed offset. That's the reason the accessor for frame_rate exists here, as far as i can see. Is this to make AVFilterLink a fully internal struct? What's the reason to do that in that case, instead of simply making its private fields actually internal and private? The idea for next bump was to clean all this mess for once and for all by deprecating all accessors, removing all the "no direct access" notes and moving all the private fields in public structs (Marked with a big "not part of the public API" warning like in the case of with AVFilterLink) into actual internal structs. That way public structs would finally become fully public and user friendly again. This change adds more abstraction and more symbols for what gain?
L'octidi 28 frimaire, an CCXXV, James Almer a écrit : > Is this to make AVFilterLink a fully internal struct? As you can see in patch 6/6 in this series. > What's the reason > to do that in that case, instead of simply making its private fields > actually internal and private? Can you suggest a method? > The idea for next bump was to clean all this mess for once and for all by Good: after this patch, there is no mess, everything is accessed through the buffersink API. > deprecating all accessors, removing all the "no direct access" notes and > moving all the private fields in public structs (Marked with a big "not > part of the public API" warning like in the case of with AVFilterLink) > into actual internal structs. By "actual internal structs", I suspect you mean something similar to this: typedef struct AVFormatContext { ... AVFormatInternal *internal; ... }; Introducing these structures was a big mistake. For the reasons, see the recent discussion about making filter_frame() non-recursive (short of it: it makes the actual code unreadable), plus another discussion I did not take part about using options on these structure (short of it: a lot of work if even possible). I do not intend to extend that mistake in libavfilter. If possible, I would rather like to kick it out, but fortunately the current uses are very limited. Regards,
On 12/18/2016 3:32 PM, Nicolas George wrote: > L'octidi 28 frimaire, an CCXXV, James Almer a écrit : >> Is this to make AVFilterLink a fully internal struct? > > As you can see in patch 6/6 in this series. > >> What's the reason >> to do that in that case, instead of simply making its private fields >> actually internal and private? > > Can you suggest a method? > >> The idea for next bump was to clean all this mess for once and for all by > > Good: after this patch, there is no mess, everything is accessed through > the buffersink API. By breaking the API, adding extra abstraction and a bunch of new symbols. You didn't answer what's the gain here. How is this better than keeping the struct public and letting library users keep accessing its fields normally? Why are you trying to make libavfilter so different than the rest? We have scheduled the deprecation and removal of /all/ accessors, and now you want to add more? If people didn't use lavfi before, they will feel less motivated to do it now since they can't even expect consistency with lavf or lavc. And those that currently do will find themselves having to adapt their programs without being given a reason as to why they are forced to, unlike with other big API changes. > >> deprecating all accessors, removing all the "no direct access" notes and >> moving all the private fields in public structs (Marked with a big "not >> part of the public API" warning like in the case of with AVFilterLink) >> into actual internal structs. > > By "actual internal structs", I suspect you mean something similar to > this: > > typedef struct AVFormatContext { > ... > AVFormatInternal *internal; > ... > }; > > Introducing these structures was a big mistake. For the reasons, see the > recent discussion about making filter_frame() non-recursive (short of > it: it makes the actual code unreadable), plus another discussion I did Back to what i said above. You're breaking API and bothering lavfi users to make internal code "more readable"? > not take part about using options on these structure (short of it: a lot > of work if even possible). > > I do not intend to extend that mistake in libavfilter. If possible, I > would rather like to kick it out, but fortunately the current uses are > very limited. I get you don't like it, but the objective should be to provide a familiar and user friendly API. Opaque internal structs don't affect users, they don't ever have to worry about them, it's one field they will never touch. Making the entire struct internal and bloating the library with accessors to write or read what has been for years and is expected to be public fields only because you want another way to hide internal state is a bit overkill and disruptively user unfriendly.
L'octidi 28 frimaire, an CCXXV, James Almer a écrit : > You didn't answer what's the gain here. Yes I did: discuss that with Hendrik, Andreas and Clément, not me. > How is this better than keeping the > struct public and letting library users keep accessing its fields normally? > Why are you trying to make libavfilter so different than the rest? We have > scheduled the deprecation and removal of /all/ accessors, and now you want > to add more? I think you did not read the code carefully enough. These accessors, on top of addressing Hendrik, Andreas and Clément's concerns, make the code actually more robust and readable. > If people didn't use lavfi before, they will feel less motivated to do it > now since they can't even expect consistency with lavf or lavc. Once again: consistency is only a means to an end. Regards,
On Sun, 18 Dec 2016 19:32:16 +0100 Nicolas George <george@nsup.org> wrote: > By "actual internal structs", I suspect you mean something similar to > this: > > typedef struct AVFormatContext { > ... > AVFormatInternal *internal; > ... > }; > > Introducing these structures was a big mistake. For the reasons, see the > recent discussion about making filter_frame() non-recursive (short of > it: it makes the actual code unreadable), plus another discussion I did > not take part about using options on these structure (short of it: a lot > of work if even possible). > > I do not intend to extend that mistake in libavfilter. If possible, I > would rather like to kick it out, but fortunately the current uses are > very limited. For buffersink, you could simply return a struct with the parameters. As a value type, it'd be a copy and wouldn't need accessors. Since you moved the discussion to general API issues... Using public fields and such "internal" structs is functionally equivalent to having an opaque struct with trivial accessors. It's basically a style issue. (The former approach, internal structs, is used and preferred for AVCodecContext etc. because it has no impact on the API.) The difference between them is essentially syntax only. Both of them have multiple disadvantages: - too much to deal with at once (whether it's fields or setters/getters), no logical separation of functionality that is lesser needed or doesn't make sense in some contexts. (Consider all these AVCodecContext fields for tuning encoders - what do they do for decoding? What do fields, which are audio-only, do if video is encoded or decoded?) - it's unclear when these fields can be set or not. (Why _can_ you set a field if the graph is already "configured"? What happens then? How is the user supposed to know when it's ok to set them?) - even if you argue that the previous point can be fixed by having the setters check the state and return an error value on misuse, it's complex for both API implementer and user - many uses of this hide internal fields from the public API user, which is good. But at the same time, this moves away from the (probably worthy) goal of allowing outside implementation of codecs, (de)muxers, filters. So I would suggest that instead of changing the whole API to use accessors, we should make the API more "transactional", and force correct use by API design. For example, we could make AVCodecContext opaque, and provide instantiation functions that take specialized structs (such as AVCodecParameters) to open the decoder. Making creation and use of an API would be a good step into improving the API IMHO. I found myself confused over what fields are always immutable in an AVHWFramesContext, and which are mutable until "init", and that's an example of the more classic mixed init/use API concept in ffmpeg.
L'octidi 28 frimaire, an CCXXV, wm4 a écrit : > For buffersink, you could simply return a struct with the parameters. > As a value type, it'd be a copy and wouldn't need accessors. You mean a single structure returned by a single accessor with all the stream's properties instead of individual accessors for individual properties? Well, the naive methods of returning a structure to the caller would make the size of the structure part of the API, but there are ways around it. I do not dislike the idea, if that is what people prefer. (AVCodecParameter would have been an obvious candidate for that, but it lacks a few necessary fields. And it has a lot of extra fields, which goes against what you write below.) > Since you moved the discussion to general API issues... > > Using public fields and such "internal" structs is functionally > equivalent to having an opaque struct with trivial accessors. It's > basically a style issue. This is true, but "functionally equivalent" is a very myopic criterion. It misses all the impact of the design on the code readability. Just look at all the "->inputs[0]" that this patch allows to eliminate (and that James missed, I think). > The difference between them is essentially syntax only. Both of them > have multiple disadvantages: > - too much to deal with at once (whether it's fields or > setters/getters), no logical separation of functionality that is > lesser needed or doesn't make sense in some contexts. (Consider all > these AVCodecContext fields for tuning encoders - what do they do for > decoding? What do fields, which are audio-only, do if video is > encoded or decoded?) It feels more a mater of documentation and auto-documentation than anything else. "There is a crf field, does it apply to the x262 encder?" and "There is a crf option, does it apply to the x262 encoder?" are exactly the same question, the only difference being that options are auto-documenting and answer the question automatically. But we could have found a way for fields as well. > - it's unclear when these fields can be set or not. (Why _can_ you set > a field if the graph is already "configured"? What happens then? How > is the user supposed to know when it's ok to set them?) > - even if you argue that the previous point can be fixed by having the > setters check the state and return an error value on misuse, it's > complex for both API implementer and user All considered, the complexity is in the task: video encoding is an incredibly complex subject. API can only do so much to ease it. > - many uses of this hide internal fields from the public API user, > which is good. But at the same time, this moves away from the > (probably worthy) goal of allowing outside implementation of > codecs, (de)muxers, filters. This calls to my recent answer to Michael about making AVFilterLink and AVFilterPad opaque: allowing foreign modules requires stability for APIs that are currently internal, and make development that much harder. > So I would suggest that instead of changing the whole API to use > accessors, we should make the API more "transactional", and force > correct use by API design. Unfortunately, if doing that was simple, we would already be doing it. Regards,
On 18.12.2016 13:22, Nicolas George wrote: > av_buffersink_get_frame_rate() did already exist; its argument becomes const. > > TODO minor version bump > > API-Change: libavfilter > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/buffersink.c | 25 +++++++++++++++++++------ > libavfilter/buffersink.h | 22 ++++++++++++++++++++-- > 2 files changed, 39 insertions(+), 8 deletions(-) > > > I think the const change is acceptable. I'm not aware of a user outside of ffmpeg and const changes shouldn't cause big problems, as only the API changes, not the ABI. So it is probably OK. > Note: I am introducing the "API-Change" Git tag: I think it will be much > more convenient than maintaining doc/APIchanges. Later I intend to write a > script using git log --grep to pretty print it. I'm not convinced that this is more convenient. APIchanges can still be changed after a commit, but the commit message can't. Also it can only replace APIchanges when everyone (including Libav) uses it. So I'd prefer if you added an APIchanges entry in addition to using this tag. Best regards, Andreas
On Mon, 19 Dec 2016 09:40:41 +0100 Nicolas George <george@nsup.org> wrote: > L'octidi 28 frimaire, an CCXXV, wm4 a écrit : > > For buffersink, you could simply return a struct with the parameters. > > As a value type, it'd be a copy and wouldn't need accessors. > > You mean a single structure returned by a single accessor with all the > stream's properties instead of individual accessors for individual > properties? Well, the naive methods of returning a structure to the > caller would make the size of the structure part of the API, but there > are ways around it. > > I do not dislike the idea, if that is what people prefer. > > (AVCodecParameter would have been an obvious candidate for that, but it > lacks a few necessary fields. And it has a lot of extra fields, which > goes against what you write below.) > > > > Since you moved the discussion to general API issues... > > > > Using public fields and such "internal" structs is functionally > > equivalent to having an opaque struct with trivial accessors. It's > > basically a style issue. > > This is true, but "functionally equivalent" is a very myopic criterion. > It misses all the impact of the design on the code readability. Just > look at all the "->inputs[0]" that this patch allows to eliminate (and > that James missed, I think). In general, replacing public fields with macro-generated accessors is really just a rename. Admittedly, it removes a confusing indirection (though ->inputs[0]) in this case, but in general the improvements are very minor. What does it matter if whether there are 100 fields or 100 set/get functions? (Didn't check whether AVCodecContext really has 100 fields, but it wouldn't surprise me.) > > The difference between them is essentially syntax only. Both of them > > have multiple disadvantages: > > - too much to deal with at once (whether it's fields or > > setters/getters), no logical separation of functionality that is > > lesser needed or doesn't make sense in some contexts. (Consider all > > these AVCodecContext fields for tuning encoders - what do they do for > > decoding? What do fields, which are audio-only, do if video is > > encoded or decoded?) > > It feels more a mater of documentation and auto-documentation than > anything else. "There is a crf field, does it apply to the x262 encder?" > and "There is a crf option, does it apply to the x262 encoder?" are > exactly the same question, the only difference being that options are > auto-documenting and answer the question automatically. But we could > have found a way for fields as well. AVOptions are unfortunately a good example how to create encoder-specific options without "polluting" the common API. I don't quite like AVOptions though, because they're too "fuzzy" for a C API (av_opt essentially emulates dynamic/weak typing to some degree), and they are typically even less well documented and defined like the C API. I don't really have a good idea how options specific to single codecs or a small number of codecs should be handled. In some cases, side-data is a good mechanism to move overly specific use-cases out of the common API, especially for decoding. If you look how Microsoft handles this, you'll see that they put additional encoder controls into separate COM interfaces, which can be not present in simpler encoders. > > - it's unclear when these fields can be set or not. (Why _can_ you set > > a field if the graph is already "configured"? What happens then? How > > is the user supposed to know when it's ok to set them?) > > - even if you argue that the previous point can be fixed by having the > > setters check the state and return an error value on misuse, it's > > complex for both API implementer and user > > All considered, the complexity is in the task: video encoding is an > incredibly complex subject. API can only do so much to ease it. > > > - many uses of this hide internal fields from the public API user, > > which is good. But at the same time, this moves away from the > > (probably worthy) goal of allowing outside implementation of > > codecs, (de)muxers, filters. > > This calls to my recent answer to Michael about making AVFilterLink and > AVFilterPad opaque: allowing foreign modules requires stability for APIs > that are currently internal, and make development that much harder. Well, I agree that neither filters nor codecs are in a state where we could easily allow foreign modules. This will require some work. > > So I would suggest that instead of changing the whole API to use > > accessors, we should make the API more "transactional", and force > > correct use by API design. > > Unfortunately, if doing that was simple, we would already be doing it. I'm just saying we're not doing it even where we could. I would claim that the right mindset is just not there.
Le decadi 30 frimaire, an CCXXV, wm4 a écrit : > > You mean a single structure returned by a single accessor with all the > > stream's properties instead of individual accessors for individual > > properties? Well, the naive methods of returning a structure to the > > caller would make the size of the structure part of the API, but there > > are ways around it. > > > > I do not dislike the idea, if that is what people prefer. You did not answer that block. Was it an oversight? > In general, replacing public fields with macro-generated accessors is > really just a rename. Yes. But I was thinking of this case specifically. In this case specifically, I think the accessors are an enhancement by themselves because they avoid digging in the filter's innards. > AVOptions are unfortunately a good example how to create > encoder-specific options without "polluting" the common API. I don't > quite like AVOptions though, because they're too "fuzzy" for a C API > (av_opt essentially emulates dynamic/weak typing to some degree), and > they are typically even less well documented and defined like the C > API. I agree on this judgement about AVOptions. > I don't really have a good idea how options specific to single codecs > or a small number of codecs should be handled. In some cases, side-data > is a good mechanism to move overly specific use-cases out of the common > API, especially for decoding. For that, I strongly disagree. Side data id an awful API. Whether it is implemented as side data or a separate field, there is an optional bit of information, and each part of the code needs to decide if it cares about it or not. As is, side data brings exactly nothing. For each AVFrameSideDataType, we could have had a pointer field in the AVFrame structure, with NULL meaning it is not present, and that would have worked exactly the same. For our needs, really exactly. That would have cost 96 octets per frame. The differences that side data brings, or could bring if we needed it, or could have brought if it was implemented properly: - Lots of accounting, memory management, unsafe casts, etc. - Impossible to use complex data structure, side data is always flat. - Side data can be repeated. But AFAIK we never use it. And fields could point to an array or a list anyway. - Side data is ordered. We do not use it. - Side data can be processed in sequence, without looking at its type. Except that to do anything useful except copying and freeing, we need to know the type. So yeah, we gain something by using side data: each time we add a new type, we gain two lines: "if (av_something_copy(&dst->field, src->field)) goto fail;" in frame_copy_props() and "av_something_freep(&frame->field)" in av_frame_unref(). Big whoop! - Side data could have been decentralized: lavc/fooenc and lavc/foodec define their own type, nobody else cares about it; lavfi/foodetect and lavfi/footweak define their own type, nobody else cares about it. Except AVFrameSideDataType is centralized. Too bad. I think some people entertained the fantasy that AVFrame could be "generic", but did not really think it through. At some point (after reinventing the options system as outlined in http://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184525.html), I will probably propose to get rid of all this, unless it has actually become useful. But it has gotten quite out of topic. > If you look how Microsoft handles this, you'll see that they put > additional encoder controls into separate COM interfaces, which can be > not present in simpler encoders. I have no idea how the COM system works, but skimming over "COM Technical Overview" from MS, I have to say: I do not want anything like that gas factory near us! > I'm just saying we're not doing it even where we could. I would claim > that the right mindset is just not there. The problem is that often APIs that "force correct use by API design" are actually "APIs that are so annoying to use that decent developers flee to other languages", the Java way. Regards,
On 20.12.2016 15:46, wm4 wrote: > In general, replacing public fields with macro-generated accessors is > really just a rename. Admittedly, it removes a confusing indirection > (though ->inputs[0]) in this case, but in general the improvements are > very minor. What does it matter if whether there are 100 fields or 100 > set/get functions? There are several benefits of using accessors: * It is much easier to keep the ABI stable for accessor functions than for public structs, because the order of members doesn't affect them. * It is much easier to check which binary uses which ABI, because the functions are listed in the symbols table, while checking which struct member is accessed requires disassembling. * Having the struct private means it can't be allocated on the stack by API users, preventing problems when the struct size changes. Best regards, Andreas
Le decadi 30 frimaire, an CCXXV, Andreas Cadhalpun a écrit : > I'm not aware of a user outside of ffmpeg and const changes shouldn't cause big > problems, as only the API changes, not the ABI. So it is probably OK. Thanks. > I'm not convinced that this is more convenient. APIchanges can still be > changed after a commit, but the commit message can't. Also it can only replace > APIchanges when everyone (including Libav) uses it. > So I'd prefer if you added an APIchanges entry in addition to using this tag. I can think of a few solutions, to merge the APIchanges and the snippets from the commit messages and allow fixes. But you are right in principle: I should not start using a feature before it is implemented. Regards,
On Tue, 20 Dec 2016 22:58:28 +0100 Nicolas George <george@nsup.org> wrote: > Le decadi 30 frimaire, an CCXXV, wm4 a écrit : > > > You mean a single structure returned by a single accessor with all the > > > stream's properties instead of individual accessors for individual > > > properties? Well, the naive methods of returning a structure to the > > > caller would make the size of the structure part of the API, but there > > > are ways around it. > > > > > > I do not dislike the idea, if that is what people prefer. > > You did not answer that block. Was it an oversight? No. I didn't see it as necessary. We seemed to be on the same page for something that could be done theoretically. > > > In general, replacing public fields with macro-generated accessors is > > really just a rename. > > Yes. But I was thinking of this case specifically. In this case > specifically, I think the accessors are an enhancement by themselves > because they avoid digging in the filter's innards. Not sure if I'd call that "innards". Though I admit that it requires some understanding how the filters are linked together. Anyway, accessing the links could be useful for inspecting other connections between filters and so on. (With your patch this whole introspection ability seems to go away? Though I'm not sure how much of it was declared public API/ABI.) > > AVOptions are unfortunately a good example how to create > > encoder-specific options without "polluting" the common API. I don't > > quite like AVOptions though, because they're too "fuzzy" for a C API > > (av_opt essentially emulates dynamic/weak typing to some degree), and > > they are typically even less well documented and defined like the C > > API. > > I agree on this judgement about AVOptions. > > > I don't really have a good idea how options specific to single codecs > > or a small number of codecs should be handled. In some cases, side-data > > is a good mechanism to move overly specific use-cases out of the common > > API, especially for decoding. > > For that, I strongly disagree. Side data id an awful API. Whether it is > implemented as side data or a separate field, there is an optional bit > of information, and each part of the code needs to decide if it cares > about it or not. > > As is, side data brings exactly nothing. For each AVFrameSideDataType, > we could have had a pointer field in the AVFrame structure, with NULL > meaning it is not present, and that would have worked exactly the same. > For our needs, really exactly. That would have cost 96 octets per > frame. Well yes, having such pointers to "optional" data would be equivalent on some higher conceptual level, just with quite different ways to use/access it in practice. IMHO the important part is that presence or absence of a whole group of fields can be signaled. Which is good for API. In general, side data can be ignored until you're looking for certain information. That's much better than just dumping everything into AVFrame, where everything demands attention at once, along with essential fields. > The differences that side data brings, or could bring if we needed it, > or could have brought if it was implemented properly: > > - Lots of accounting, memory management, unsafe casts, etc. > > - Impossible to use complex data structure, side data is always flat. > > - Side data can be repeated. But AFAIK we never use it. And fields could > point to an array or a list anyway. > > - Side data is ordered. We do not use it. > > - Side data can be processed in sequence, without looking at its type. > Except that to do anything useful except copying and freeing, we need > to know the type. > > So yeah, we gain something by using side data: each time we add a new > type, we gain two lines: > "if (av_something_copy(&dst->field, src->field)) goto fail;" in > frame_copy_props() and "av_something_freep(&frame->field)" in > av_frame_unref(). Big whoop! > > - Side data could have been decentralized: lavc/fooenc and lavc/foodec > define their own type, nobody else cares about it; lavfi/foodetect and > lavfi/footweak define their own type, nobody else cares about it. > Except AVFrameSideDataType is centralized. Too bad. > > I think some people entertained the fantasy that AVFrame could be > "generic", but did not really think it through. I agree about those points. I particularly dislike that we can't seem to decide whether side data should be a bytestream (defined explicitly, like a file format) or the contents of a struct (defined by C API/ABI). You forgot to mention the code duplication between frame/packet/stream side data management. > At some point (after reinventing the options system as outlined in > http://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184525.html), I > will probably propose to get rid of all this, unless it has actually > become useful. > > But it has gotten quite out of topic. > > > If you look how Microsoft handles this, you'll see that they put > > additional encoder controls into separate COM interfaces, which can be > > not present in simpler encoders. > > I have no idea how the COM system works, but skimming over "COM > Technical Overview" from MS, I have to say: I do not want anything like > that gas factory near us! I think you're not dismissing it too early. It provides some interesting concepts in term of ABI and API compatibility. Although it's terrible to use, there's something that can be learned from it. To make it short, everything in COM consists of structs with function pointers. Structs are never extended, if you need new function pointers, you just add new structs, which you can dynamically query from the old ones. This gives you 100% ABI downwards and upwards compatibility. You also don't have to worry about linking to functions not present in older Windows or whatever versions, because structs with function pointers are a compile-time thing. You will merely get a runtime failure when trying to query support for an unsupported struct. (I tried to avoid COM terms, but COM calls these structs "interfaces".) It's a pretty fascinating contrast to the fuckhackery we do with extending structs while trying to keep ABI compatibility, version and configure checks in API user code when trying to stay compatible with multiple libavcodec versions, etc. > > I'm just saying we're not doing it even where we could. I would claim > > that the right mindset is just not there. > > The problem is that often APIs that "force correct use by API design" > are actually "APIs that are so annoying to use that decent developers > flee to other languages", the Java way. Not necessarily. Java API design might serve as negative example, sure.
On Wed, 21 Dec 2016 01:56:59 +0100 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com> wrote: > On 20.12.2016 15:46, wm4 wrote: > > In general, replacing public fields with macro-generated accessors is > > really just a rename. Admittedly, it removes a confusing indirection > > (though ->inputs[0]) in this case, but in general the improvements are > > very minor. What does it matter if whether there are 100 fields or 100 > > set/get functions? > > There are several benefits of using accessors: > * It is much easier to keep the ABI stable for accessor functions than > for public structs, because the order of members doesn't affect them. > * It is much easier to check which binary uses which ABI, because the > functions are listed in the symbols table, while checking which > struct member is accessed requires disassembling. > * Having the struct private means it can't be allocated on the stack > by API users, preventing problems when the struct size changes. Acknowledged. I was talking mostly about API.
L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit : > +AVRational av_buffersink_get_frame_rate (const AVFilterContext *ctx); > +int av_buffersink_get_w (const AVFilterContext *ctx); > +int av_buffersink_get_h (const AVFilterContext *ctx); > +AVRational av_buffersink_get_sample_aspect_ratio (const AVFilterContext *ctx); So, I ask this of everybody who care: what API do you prefer? This one, i.e.: encoder->width = av_buffersink_get_w(sink); encoder->height = av_buffersink_get_h(sink); encoder->sample_aspect_ratio = av_buffersink_get_sample_aspect_ratio(sink); Or one with a single access to all the properties: const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink); encoder->width = fmt->w; encoder->height = fmt->h; encoder->sample_aspect_ratio = fmt->sample_aspect_ratio; Regards,
Le primidi 1er nivôse, an CCXXV, wm4 a écrit : [about windows COM system] > To make it short, everything in COM consists of structs with function > pointers. Structs are never extended, if you need new function > pointers, you just add new structs, which you can dynamically query > from the old ones. This gives you 100% ABI downwards and upwards > compatibility. You also don't have to worry about linking to functions > not present in older Windows or whatever versions, because structs with > function pointers are a compile-time thing. You will merely get a > runtime failure when trying to query support for an unsupported struct. > (I tried to avoid COM terms, but COM calls these structs "interfaces".) > > It's a pretty fascinating contrast to the fuckhackery we do with > extending structs while trying to keep ABI compatibility, version > and configure checks in API user code when trying to stay compatible > with multiple libavcodec versions, etc. It is probably a large part of the reason windows is slow on tomorrow's computers while Linux and BSD are fast on yesterday's. Regards,
On Fri, Dec 23, 2016 at 03:31:45PM +0100, Nicolas George wrote: > L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit : > > +AVRational av_buffersink_get_frame_rate (const AVFilterContext *ctx); > > +int av_buffersink_get_w (const AVFilterContext *ctx); > > +int av_buffersink_get_h (const AVFilterContext *ctx); > > +AVRational av_buffersink_get_sample_aspect_ratio (const AVFilterContext *ctx); > > So, I ask this of everybody who care: what API do you prefer? > > This one, i.e.: > > encoder->width = av_buffersink_get_w(sink); > encoder->height = av_buffersink_get_h(sink); > encoder->sample_aspect_ratio = av_buffersink_get_sample_aspect_ratio(sink); > > Or one with a single access to all the properties: > > const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink); > encoder->width = fmt->w; > encoder->height = fmt->h; > encoder->sample_aspect_ratio = fmt->sample_aspect_ratio; From these 2 the first but i think the user app needs more access to be able to implement filters and this could make either API obsoleete [...]
On Fri, Dec 23, 2016 at 04:59:45PM +0100, Michael Niedermayer wrote: > On Fri, Dec 23, 2016 at 03:31:45PM +0100, Nicolas George wrote: > > L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit : > > > +AVRational av_buffersink_get_frame_rate (const AVFilterContext *ctx); > > > +int av_buffersink_get_w (const AVFilterContext *ctx); > > > +int av_buffersink_get_h (const AVFilterContext *ctx); > > > +AVRational av_buffersink_get_sample_aspect_ratio (const AVFilterContext *ctx); > > > > So, I ask this of everybody who care: what API do you prefer? > > > > This one, i.e.: > > > > encoder->width = av_buffersink_get_w(sink); > > encoder->height = av_buffersink_get_h(sink); > > encoder->sample_aspect_ratio = av_buffersink_get_sample_aspect_ratio(sink); > > > > Or one with a single access to all the properties: > > > > const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink); > > encoder->width = fmt->w; > > encoder->height = fmt->h; > > encoder->sample_aspect_ratio = fmt->sample_aspect_ratio; > > From these 2 the first but i think the user app needs more access > to be able to implement filters and this could make either API > obsoleete also AVCodecParameters would be an option to use as a struct if a struct is used, the lack of AVClass/AVOption in it may cause problems though when lib versions differ and field have been added between [...]
Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > From these 2 the first Ok, that makes one. > but i think the user app needs more access > to be able to implement filters and this could make either API > obsoleete User app implementing filters is not for today nor tomorrow. I think it is better to start thinking on how to cross that bridge when we start seeing it on the horizon. Regards,
Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > also AVCodecParameters would be an option to use as a struct if a > struct is used Unfortunately not, since it lacks at least time_base and frame_rate. We could add them, but think it would be a bad idea. It also has a lot of other fields that are of no use for this case, that would need documentation. Regards,
On Fri, 23 Dec 2016 15:49:16 +0100 Nicolas George <george@nsup.org> wrote: > Le primidi 1er nivôse, an CCXXV, wm4 a écrit : > > [about windows COM system] > > > To make it short, everything in COM consists of structs with function > > pointers. Structs are never extended, if you need new function > > pointers, you just add new structs, which you can dynamically query > > from the old ones. This gives you 100% ABI downwards and upwards > > compatibility. You also don't have to worry about linking to functions > > not present in older Windows or whatever versions, because structs with > > function pointers are a compile-time thing. You will merely get a > > runtime failure when trying to query support for an unsupported struct. > > (I tried to avoid COM terms, but COM calls these structs "interfaces".) > > > > It's a pretty fascinating contrast to the fuckhackery we do with > > extending structs while trying to keep ABI compatibility, version > > and configure checks in API user code when trying to stay compatible > > with multiple libavcodec versions, etc. > > It is probably a large part of the reason windows is slow on tomorrow's > computers while Linux and BSD are fast on yesterday's. No, it's not.
On 12/23/2016 11:31 AM, Nicolas George wrote: > L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit : >> +AVRational av_buffersink_get_frame_rate (const AVFilterContext *ctx); >> +int av_buffersink_get_w (const AVFilterContext *ctx); >> +int av_buffersink_get_h (const AVFilterContext *ctx); >> +AVRational av_buffersink_get_sample_aspect_ratio (const AVFilterContext *ctx); > > So, I ask this of everybody who care: what API do you prefer? > > This one, i.e.: > > encoder->width = av_buffersink_get_w(sink); > encoder->height = av_buffersink_get_h(sink); > encoder->sample_aspect_ratio = av_buffersink_get_sample_aspect_ratio(sink); > > Or one with a single access to all the properties: > > const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink); > encoder->width = fmt->w; > encoder->height = fmt->h; > encoder->sample_aspect_ratio = fmt->sample_aspect_ratio; > > Regards, I very much prefer the latter. Only one symbol, a (hopefully) easily extensible struct if needed, etc.
On Fri, Dec 23, 2016 at 05:08:49PM +0100, Nicolas George wrote: > Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > > From these 2 the first > > Ok, that makes one. > > > but i think the user app needs more access > > to be able to implement filters and this could make either API > > obsoleete > > User app implementing filters is not for today nor tomorrow. I think it > is better to start thinking on how to cross that bridge when we start > seeing it on the horizon. libavfilter had a public API that allowed filters to be implemented these changes move it farther away from it I do care about having some public API that allows filters to be implemented. You pointed out that you work alone and people dont review your lavfi patches. I think if you integrate others goals more people would For example your plans can stay the same but include the declared goal of making the API public once they are all done. And suddenly i would be interrested in this. Of course it doesnt make days have more hours or my todo shorter but pushing the public API away makes me loose interrest for sure. Other people might have other ideas and goals too, which they want lavfi to achieve. If more things can be integrated more people might contribute ... thx [...]
Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > libavfilter had a public API that allowed filters to be implemented > these changes move it farther away from it > I do care about having some public API that allows filters > to be implemented. > You pointed out that you work alone and people dont review your lavfi > patches. I think if you integrate others goals more people would > > For example your plans can stay the same but include the declared > goal of making the API public once they are all done. And suddenly > i would be interrested in this. Of course it doesnt make days > have more hours or my todo shorter but pushing the public API away > makes me loose interrest for sure. > > Other people might have other ideas and goals too, which they want > lavfi to achieve. > If more things can be integrated more people might contribute ... You misunderstood: I am not excluding an API to allow external filters, I am saying that it is too soon to think about it. The way filters must be written is in a complete rework. Once it is stabilized, tested extensively, we know it will not changing again, then we can make parts of it public to allow external filters. Anything else would be a compatibility nightmare. Do you not agree? Regards,
On Fri, Dec 23, 2016 at 06:51:38PM +0100, Nicolas George wrote: > Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > > libavfilter had a public API that allowed filters to be implemented > > these changes move it farther away from it > > I do care about having some public API that allows filters > > to be implemented. > > You pointed out that you work alone and people dont review your lavfi > > patches. I think if you integrate others goals more people would > > > > For example your plans can stay the same but include the declared > > goal of making the API public once they are all done. And suddenly > > i would be interrested in this. Of course it doesnt make days > > have more hours or my todo shorter but pushing the public API away > > makes me loose interrest for sure. > > > > Other people might have other ideas and goals too, which they want > > lavfi to achieve. > > If more things can be integrated more people might contribute ... > > You misunderstood: I am not excluding an API to allow external filters, > I am saying that it is too soon to think about it. The way filters must > be written is in a complete rework. Once it is stabilized, tested > extensively, we know it will not changing again, then we can make parts > of it public to allow external filters. > > Anything else would be a compatibility nightmare. Do you not agree? APIs in FFmpeg will change as long as the project is alive. new developers join, older ones leave, peoples goals and oppinions change. The libavfilter API is based on the lessons learned from previous projects and frameworks, in that way this codebase has a quite long timeline and many experienced developers from multiple other free software projects upon whos shoulders this rests in some sense. If we only make the API public once the ultimate global optimum is reached we will never do so. What iam asking for is not that you declare a API public that you intend to change but that we commit to declaring the API public in the reachable future and not as a goalline that keeps shifting farther into the future forever. you may eventually be happy with the API in libavfilter but by the time others with other goals and oppinions will just start their journy. Also making the API public should be a real goal and be considered in the steps taken For example if AVFilterLink isnt intended to be substantially changed and is intended to be public when the API becomes public then i dont see why it should not stay public making it private, rewritig code, adding public accessors just to then make it public again and deprecate the accessors is a huge amount of wasted time. [...]
Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > APIs in FFmpeg will change as long as the project is alive. > > new developers join, older ones leave, peoples goals and oppinions > change. The libavfilter API is based on the lessons learned from > previous projects and frameworks, in that way this codebase has a quite > long timeline and many experienced developers from multiple other > free software projects upon whos shoulders this rests in some sense. > > If we only make the API public once the ultimate global optimum is > reached we will never do so. I was not referring to a hypothetical global optimum, but right now we are nowhere near a local optimum: you have certainly noticed that since I pushed the 45k patch making filter_frame non-recursive, I had to fix several bugs. Well, there are still a few of them lurking, and then we need to reap the benefits of the design change. Two days ago, I outlined my plans for lavfi: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/204686.html I say: first do all these points, because any of these might require a surprise rework of some internal API. Then (in parallel when relevant), adapt ffmpeg.c to work with the cleaned-up API of lavfi and fix the scheduling. Then wait a year, to be sure. Then start working on external filters. Does it seem unreasonable? I can assure you, when the last points in the plan will be done, I will be so fed up with it that I will easily leave the API alone for a year. There are plenty of other parts of the code I would like to work on. For reference, I am halfway through the second item in the plan. Probably more than halfway, actually. Regards,
On Fri, Dec 23, 2016 at 09:23:50PM +0100, Nicolas George wrote: > Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > > APIs in FFmpeg will change as long as the project is alive. > > > > new developers join, older ones leave, peoples goals and oppinions > > change. The libavfilter API is based on the lessons learned from > > previous projects and frameworks, in that way this codebase has a quite > > long timeline and many experienced developers from multiple other > > free software projects upon whos shoulders this rests in some sense. > > > > If we only make the API public once the ultimate global optimum is > > reached we will never do so. > > I was not referring to a hypothetical global optimum, but right now we > are nowhere near a local optimum: you have certainly noticed that since > I pushed the 45k patch making filter_frame non-recursive, I had to fix > several bugs. Well, there are still a few of them lurking, and then we > need to reap the benefits of the design change. > > Two days ago, I outlined my plans for lavfi: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/204686.html > > I say: first do all these points, because any of these might require a > surprise rework of some internal API. Then (in parallel when relevant), > adapt ffmpeg.c to work with the cleaned-up API of lavfi and fix the > scheduling. > > Then wait a year, to be sure. > > Then start working on external filters. > > Does it seem unreasonable? shouldnt there be a public annoncement about the intend to make the API public shouldnt there be a public call for API design suggestions and discussion shouldnt there be a public call for API related patches with deadline shouldnt there be a go/no-go poll of the FFmpeg developers I dont think a wait period makes sense. By the time everyone is ok with making the API public the code will have been more then extensivly tested and waiting further would likely add little [...]
Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > shouldnt there be a public annoncement about the intend to make the API public > shouldnt there be a public call for API design suggestions and discussion > shouldnt there be a public call for API related patches with deadline > shouldnt there be a go/no-go poll of the FFmpeg developers I am not sure what you are angling at here. I announced my projects to rework the internal workings and global API when I started working on making request_frame non-recursive (note: request_frame, not filter_frame; this patch landed more than a year ago) and mentioned them several times since then, for example when Paul considered working on threading IIRC. I did not gave that much details because nobody seemed interested in them. And of course, no significant change in API or design went in without staying for quite a time on the mailing-list. > I dont think a wait period makes sense. By the time everyone is ok > with making the API public the code will have been more then > extensivly tested and waiting further would likely add little So you are saying that the wait period will take care of itself even if we do not decide to enforce it. I am ok with it. And, after all, what do you want, concretely? I, personally, am not working on making external filters possible. But I am not preventing it either. (Well, this patch makes it a little bit more work, but just work, not hard.) And I think that when I am done with my plan, it will be actually easier because a cleaner internal API is easier to make public. Another point: this particular patch, and the series that surrounds it, I do not really want it. For all I care, we could drop it (either all the series, or just 5-6 because 1-4 make the code clearer). On the other hand, Andreas, Clément, Hendrik and James would not be happy, because they insisted to hide the innards of AVFilterLink more deeply after the filter_frame patch. This was discussed on the mailing list. I probably should let you discuss the issue with them. The code is written, it did not take much time and does not interact with my current work: it can be applied or dropped indifferently. Regards,
On Fri, Dec 23, 2016 at 11:46:07PM +0100, Nicolas George wrote: > Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > > shouldnt there be a public annoncement about the intend to make the API public > > shouldnt there be a public call for API design suggestions and discussion > > shouldnt there be a public call for API related patches with deadline > > shouldnt there be a go/no-go poll of the FFmpeg developers > > I am not sure what you are angling at here. I announced my projects to > rework the internal workings and global API when I started working on > making request_frame non-recursive (note: request_frame, not > filter_frame; this patch landed more than a year ago) and mentioned them > several times since then, for example when Paul considered working on > threading IIRC. I did not gave that much details because nobody seemed > interested in them. > > And of course, no significant change in API or design went in without > staying for quite a time on the mailing-list. you snipped the context and reply to the quote as if it was meant in a different context. Not everyone intends to attack you :) The quoted text are the steps which IMHO make sense to design a API and to then make it public. What you talk about are the steps that make sense to get the changes you want in. theres nothing wrong with that it just has a differnt goal and i thought we talk about the other. > > > I dont think a wait period makes sense. By the time everyone is ok > > with making the API public the code will have been more then > > extensivly tested and waiting further would likely add little > > So you are saying that the wait period will take care of itself even if > we do not decide to enforce it. I am ok with it. > > And, after all, what do you want, concretely? as you ask ... Id like to see libavfilter be less centralized, id like to be able to write a filter and know that if it gets rejected on ffmpeg-dev that i can still publish it on my own git repo and everyone could easily use it. (as with plugins which could have bin packages in distros for easy use not requireng users to patch / merge source) This is an especially annoying situation when theres a consulting job and then someone decides to demand some change which practically is preventing the actual usecase one is paid for to be solved. If its not a consulting job droping some patch isnt a huge problem if a reviewer is stubborn and insists on something which is unpractical. But if one is doing the work for someone else, droping a patch is much more annoying. Being able to create a plugin sidesteps this. > > I, personally, am not working on making external filters possible. But I > am not preventing it either. (Well, this patch makes it a little bit > more work, but just work, not hard.) And I think that when I am done > with my plan, it will be actually easier because a cleaner internal API > is easier to make public. > > Another point: this particular patch, and the series that surrounds it, > I do not really want it. For all I care, we could drop it (either all > the series, or just 5-6 because 1-4 make the code clearer). > > On the other hand, Andreas, Clément, Hendrik and James would not be > happy, because they insisted to hide the innards of AVFilterLink more > deeply after the filter_frame patch. This was discussed on the mailing > list. I probably should let you discuss the issue with them. The code is > written, it did not take much time and does not interact with my current > work: it can be applied or dropped indifferently. My oppinion is that If we intend to have AVFilterLink public in the future then making it private now is a bad idea and wasted time. [...]
Le quartidi 4 nivôse, an CCXXV, Michael Niedermayer a écrit : > you snipped the context and reply to the quote as if it was meant > in a different context. > Not everyone intends to attack you :) Sorry; I might have still been a little on edge. > The quoted text are the steps which IMHO make sense to design a > API and to then make it public. What you talk about are the steps that > make sense to get the changes you want in. > theres nothing wrong with that it just has a differnt goal and i > thought we talk about the other. Oh, ok. But the changes that I want in will change significantly how filters must be written, and therefore have a big impact on the necessary API. > as you ask ... > > Id like to see libavfilter be less centralized, id like to be able > to write a filter and know that if it gets rejected on ffmpeg-dev that > i can still publish it on my own git repo and everyone could easily use > it. (as with plugins which could have bin packages in distros for easy > use not requireng users to patch / merge source) > This is an especially annoying situation when theres a consulting job > and then someone decides to demand some change which practically is > preventing the actual usecase one is paid for to be solved. > > If its not a consulting job droping some patch isnt a huge > problem if a reviewer is stubborn and insists on something which is > unpractical. > But if one is doing the work for someone else, droping a patch is much > more annoying. Being able to create a plugin sidesteps this. In the long term, I agree. > My oppinion is that If we intend to have AVFilterLink public in the > future then making it private now is a bad idea and wasted time. As I said, I am not the one who wants to make AVFilterLink opaque. Note that making it visible again when it is ready is a matter of minutes. Regards,
Le quartidi 4 nivôse, an CCXXV, Michael Niedermayer a écrit : > My oppinion is that If we intend to have AVFilterLink public in the > future then making it private now is a bad idea and wasted time. Something I just remembered: With the prospect of inter-filters threading, it is better if even our own filters do not access links directly, because then it is only necessary to add synchronization in the helper functions. What about moving the definition in its own header? For compatibility at the beginning, this header can be included by avfilter.h, but then it can either be made private or stay there to be included by the parts of the code that really need it. Regards,
On Fri, 23 Dec 2016 21:02:09 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Dec 23, 2016 at 06:51:38PM +0100, Nicolas George wrote: > > Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit : > > > libavfilter had a public API that allowed filters to be implemented > > > these changes move it farther away from it > > > I do care about having some public API that allows filters > > > to be implemented. > > > You pointed out that you work alone and people dont review your lavfi > > > patches. I think if you integrate others goals more people would > > > > > > For example your plans can stay the same but include the declared > > > goal of making the API public once they are all done. And suddenly > > > i would be interrested in this. Of course it doesnt make days > > > have more hours or my todo shorter but pushing the public API away > > > makes me loose interrest for sure. > > > > > > Other people might have other ideas and goals too, which they want > > > lavfi to achieve. > > > If more things can be integrated more people might contribute ... > > > > You misunderstood: I am not excluding an API to allow external filters, > > I am saying that it is too soon to think about it. The way filters must > > be written is in a complete rework. Once it is stabilized, tested > > extensively, we know it will not changing again, then we can make parts > > of it public to allow external filters. > > > > Anything else would be a compatibility nightmare. Do you not agree? > > APIs in FFmpeg will change as long as the project is alive. > A public API for something as complicated and far-reaching as external filters or codecs should be tiny and concise, not expose the current internal API-vomit we have to the world.
diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c index 7b7b477..030ca80 100644 --- a/libavfilter/buffersink.c +++ b/libavfilter/buffersink.c @@ -279,14 +279,27 @@ void av_buffersink_set_frame_size(AVFilterContext *ctx, unsigned frame_size) inlink->partial_buf_size = frame_size; } -AVRational av_buffersink_get_frame_rate(AVFilterContext *ctx) -{ - av_assert0( !strcmp(ctx->filter->name, "buffersink") - || !strcmp(ctx->filter->name, "ffbuffersink")); - - return ctx->inputs[0]->frame_rate; +#define MAKE_AVFILTERLINK_ACCESSOR(type, field) \ +type av_buffersink_get_##field(const AVFilterContext *ctx) { \ + av_assert0(ctx->filter->uninit == uninit); \ + return ctx->inputs[0]->field; \ } +MAKE_AVFILTERLINK_ACCESSOR(enum AVMediaType , type ); +MAKE_AVFILTERLINK_ACCESSOR(AVRational , time_base ); +MAKE_AVFILTERLINK_ACCESSOR(int , format ); + +MAKE_AVFILTERLINK_ACCESSOR(AVRational , frame_rate ); +MAKE_AVFILTERLINK_ACCESSOR(int , w ); +MAKE_AVFILTERLINK_ACCESSOR(int , h ); +MAKE_AVFILTERLINK_ACCESSOR(AVRational , sample_aspect_ratio); + +MAKE_AVFILTERLINK_ACCESSOR(int , channels ); +MAKE_AVFILTERLINK_ACCESSOR(uint64_t , channel_layout ); +MAKE_AVFILTERLINK_ACCESSOR(int , sample_rate ); + +MAKE_AVFILTERLINK_ACCESSOR(AVBufferRef * , hw_frames_ctx ); + static av_cold int vsink_init(AVFilterContext *ctx, void *opaque) { BufferSinkContext *buf = ctx->priv; diff --git a/libavfilter/buffersink.h b/libavfilter/buffersink.h index e399b91..f51fa7c 100644 --- a/libavfilter/buffersink.h +++ b/libavfilter/buffersink.h @@ -101,9 +101,27 @@ AVABufferSinkParams *av_abuffersink_params_alloc(void); void av_buffersink_set_frame_size(AVFilterContext *ctx, unsigned frame_size); /** - * Get the frame rate of the input. + * @defgroup lavfi_buffersink_accessors Buffer sink accessors + * Get the properties of the stream + * @{ */ -AVRational av_buffersink_get_frame_rate(AVFilterContext *ctx); + +enum AVMediaType av_buffersink_get_type (const AVFilterContext *ctx); +AVRational av_buffersink_get_time_base (const AVFilterContext *ctx); +int av_buffersink_get_format (const AVFilterContext *ctx); + +AVRational av_buffersink_get_frame_rate (const AVFilterContext *ctx); +int av_buffersink_get_w (const AVFilterContext *ctx); +int av_buffersink_get_h (const AVFilterContext *ctx); +AVRational av_buffersink_get_sample_aspect_ratio (const AVFilterContext *ctx); + +int av_buffersink_get_channels (const AVFilterContext *ctx); +uint64_t av_buffersink_get_channel_layout (const AVFilterContext *ctx); +int av_buffersink_get_sample_rate (const AVFilterContext *ctx); + +AVBufferRef * av_buffersink_get_hw_frames_ctx (const AVFilterContext *ctx); + +/** @} */ /** * Get a frame with filtered data from sink and put it in frame.
av_buffersink_get_frame_rate() did already exist; its argument becomes const. TODO minor version bump API-Change: libavfilter Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/buffersink.c | 25 +++++++++++++++++++------ libavfilter/buffersink.h | 22 ++++++++++++++++++++-- 2 files changed, 39 insertions(+), 8 deletions(-) I think the const change is acceptable. Note: I am introducing the "API-Change" Git tag: I think it will be much more convenient than maintaining doc/APIchanges. Later I intend to write a script using git log --grep to pretty print it.