| Message ID | 20161229143403.2851-1-george@nsup.org |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Dec 29, 2016 at 03:33:47PM +0100, Nicolas George wrote: > Also introduce libavfilter/filters.h for all functions needed > to implement filters. > > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/avfilter.c | 14 ++++++++++++++ > libavfilter/filters.h | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > create mode 100644 libavfilter/filters.h > > > The series is still based on the series about AVFilterLink, although it does > not actually depends on it, it is just a matter of tiny conflicts in the > headers. > > Still passes FATE at each step. > > Changes in this commit: rename ff_link -> ff_inlink and move to filters.h, > and always return the status (avoids accessing the link directly). > > > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c > index 8d106c1ab3..47a48998e0 100644 > --- a/libavfilter/avfilter.c > +++ b/libavfilter/avfilter.c > @@ -38,6 +38,7 @@ > #include "audio.h" > #include "avfilter.h" > #include "avfilterlink.h" > +#include "filters.h" > #include "formats.h" > #include "internal.h" > > @@ -1542,6 +1543,19 @@ int ff_filter_activate(AVFilterContext *filter) > return ret; > } > > +int ff_inlink_acknowledge_status(AVFilterLink *link, int *rstatus) > +{ > + if (ff_framequeue_queued_frames(&link->fifo)) > + return *rstatus = 0; > + if (link->status_out) > + return *rstatus = link->status_out; > + if (!link->status_in) > + return *rstatus = 0; > + *rstatus = link->status_out = link->status_in; > + ff_update_link_current_pts(link, link->status_in_pts); > + return 1; > +} > + > const AVClass *avfilter_get_class(void) > { > return &avfilter_class; > diff --git a/libavfilter/filters.h b/libavfilter/filters.h > new file mode 100644 > index 0000000000..f4a46a023c > --- /dev/null > +++ b/libavfilter/filters.h > @@ -0,0 +1,37 @@ > +/* > + * Filters implementation helper functions > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public License > + * as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public License > + * along with FFmpeg; if not, write to the Free Software Foundation, Inc., > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#ifndef AVFILTER_FILTERS_H > +#define AVFILTER_FILTERS_H > + > +/** > + * Filters implementation helper functions > + */ > + > +#include "avfilter.h" > + > +/** > + * Test and acknowledge the change of status on the link. > + * @param[out] new or current status > + * @return >0 if status changed, <0 if status already acked, 0 otherwise > + */ This is a bit terse it does not explain what a status is or where that is documented or what acknowledging means or when it should be called [...]
On Thu, 29 Dec 2016 15:33:47 +0100 Nicolas George <george@nsup.org> wrote: > Also introduce libavfilter/filters.h for all functions needed > to implement filters. > > Signed-off-by: Nicolas George <george@nsup.org> Can we have a document in doc/ or a doxygen section that gives an overview how the "new" internal API works?
Le decadi 20 nivôse, an CCXXV, wm4 a écrit : > Can we have a document in doc/ or a doxygen section that gives an > overview how the "new" internal API works? I already stated several times this was planned. BTW, « API-vomit » is not acceptable IMHO.
On Mon, 9 Jan 2017 11:48:50 +0100 Nicolas George <george@nsup.org> wrote: > Le decadi 20 nivôse, an CCXXV, wm4 a écrit : > > Can we have a document in doc/ or a doxygen section that gives an > > overview how the "new" internal API works? > > I already stated several times this was planned. Do you have any explanation as to why you pushed this patchset without doing what was requested? > BTW, « API-vomit » is not acceptable IMHO. >
Le tridi 23 nivôse, an CCXXV, wm4 a écrit : > Do you have any explanation as to why you pushed this patchset without > doing what was requested? What do you mean exactly?
On Thu, 12 Jan 2017 14:22:56 +0100 Nicolas George <george@nsup.org> wrote: > Le tridi 23 nivôse, an CCXXV, wm4 a écrit : > > Do you have any explanation as to why you pushed this patchset without > > doing what was requested? > > What do you mean exactly? > In my opinion, such an overview was necessary along such a huge change - if only to make sure reviewers can properly review the new API before giving their ok.
Le tridi 23 nivôse, an CCXXV, wm4 a écrit : > In my opinion, such an overview was necessary along such a huge > change - if only to make sure reviewers can properly review the new > API before giving their ok. That "overview" is already present on the mailing-list and in the comments of the patches, so there is no problem. Thanks for clarifying, and no thanks for wasting my time.
On Thu, 12 Jan 2017 14:49:51 +0100 Nicolas George <george@nsup.org> wrote: > Le tridi 23 nivôse, an CCXXV, wm4 a écrit : > > In my opinion, such an overview was necessary along such a huge > > change - if only to make sure reviewers can properly review the new > > API before giving their ok. > > That "overview" is already present on the mailing-list and in the > comments of the patches, so there is no problem. Thanks for clarifying, Well then why didn't you point these out when I asked? Oh I know, these comments didn't really give a coherent overview. > and no thanks for wasting my time. If the patch review process is too time consuming for you, you're welcome to contribute to other projects instead.
Le tridi 23 nivôse, an CCXXV, wm4 a écrit : > Well then why didn't you point these out when I asked? Oh I know, these > comments didn't really give a coherent overview. You do not know that, you only read the patches looking for ways to criticize and nag me. Never to do constructive comments. > If the patch review process is too time consuming for you, you're > welcome to contribute to other projects instead. What "review"? Do not make me laugh. I will no longer reply to you on this subject. If other people have the impression that I am unjust with wm4, please let me know.
On Thu, 12 Jan 2017 15:04:04 +0100 Nicolas George <george@nsup.org> wrote: > Le tridi 23 nivôse, an CCXXV, wm4 a écrit : > > Well then why didn't you point these out when I asked? Oh I know, these > > comments didn't really give a coherent overview. > > You do not know that, you only read the patches looking for ways to > criticize and nag me. Never to do constructive comments. I'm sorry that you see absolutely everything that comes from me as an attack. > > If the patch review process is too time consuming for you, you're > > welcome to contribute to other projects instead. > > What "review"? Do not make me laugh. Well, I didn't even get the chance. Do you expect us to reverse-engineer your patches? This was 17 parts patch series, and I don't think I asked for too much.
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 8d106c1ab3..47a48998e0 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -38,6 +38,7 @@ #include "audio.h" #include "avfilter.h" #include "avfilterlink.h" +#include "filters.h" #include "formats.h" #include "internal.h" @@ -1542,6 +1543,19 @@ int ff_filter_activate(AVFilterContext *filter) return ret; } +int ff_inlink_acknowledge_status(AVFilterLink *link, int *rstatus) +{ + if (ff_framequeue_queued_frames(&link->fifo)) + return *rstatus = 0; + if (link->status_out) + return *rstatus = link->status_out; + if (!link->status_in) + return *rstatus = 0; + *rstatus = link->status_out = link->status_in; + ff_update_link_current_pts(link, link->status_in_pts); + return 1; +} + const AVClass *avfilter_get_class(void) { return &avfilter_class; diff --git a/libavfilter/filters.h b/libavfilter/filters.h new file mode 100644 index 0000000000..f4a46a023c --- /dev/null +++ b/libavfilter/filters.h @@ -0,0 +1,37 @@ +/* + * Filters implementation helper functions + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public License + * as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with FFmpeg; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef AVFILTER_FILTERS_H +#define AVFILTER_FILTERS_H + +/** + * Filters implementation helper functions + */ + +#include "avfilter.h" + +/** + * Test and acknowledge the change of status on the link. + * @param[out] new or current status + * @return >0 if status changed, <0 if status already acked, 0 otherwise + */ +int ff_inlink_acknowledge_status(AVFilterLink *link, int *rstatus); + +#endif /* AVFILTER_FILTERS_H */
Also introduce libavfilter/filters.h for all functions needed to implement filters. Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/avfilter.c | 14 ++++++++++++++ libavfilter/filters.h | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 libavfilter/filters.h The series is still based on the series about AVFilterLink, although it does not actually depends on it, it is just a matter of tiny conflicts in the headers. Still passes FATE at each step. Changes in this commit: rename ff_link -> ff_inlink and move to filters.h, and always return the status (avoids accessing the link directly).