Re: [FFmpeg-devel] [PATCH 1/4] lavfi: add filter metaframes infrastructure.
Le quintidi 15 thermidor, an CCXXII, Stefano Sabatini a écrit : > It shall probably execute the remaining part of the function even in > case of failure. I am not completely sure about that. AFAIK, we consider most filtering failures fatal at some point or another anyway. > I find this a bit confusing. Can you explain why the NOP metaframe is > needed? Depending on the type of the message and the default action taken, filters may need to inhibit the default action because they already did the work or changed something else somewhere that would conflict. I can remove that hunk and wait for something to actually use it if you prefer. > LGTM otherwise. Thanks to everyone for the reviews. New series posted. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] lavfi: add filter metaframes infrastructure.
On date Wednesday 2014-07-30 23:44:46 +0200, Nicolas George encoded:
> Metaframes are frames without data, identified by a negative
> format code, used to carry special conditions.
> They are sent only to filter that declare supporting them.
> The only metaframe for now is EOF; this mechanism augments
> the current mechanism based on request_frame() returning
> AVERROR_EOF, with the advantage that the EOF metaframe carries
> a timestamp.
> The metaframes are a purely internal API and do not leak to
> the application.
>
> Signed-off-by: Nicolas George
> ---
> libavfilter/avfilter.c | 73
> +-
> libavfilter/internal.h | 34 +++
> 2 files changed, 100 insertions(+), 7 deletions(-)
>
>
> Changed the name to "metaframes".
> Allocate the frame with the classic method.
>
>
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 7b11467..7894173 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -346,15 +346,16 @@ int ff_request_frame(AVFilterLink *link)
> ret = link->srcpad->request_frame(link);
> else if (link->src->inputs[0])
> ret = ff_request_frame(link->src->inputs[0]);
> -if (ret == AVERROR_EOF && link->partial_buf) {
> -AVFrame *pbuf = link->partial_buf;
> -link->partial_buf = NULL;
> -ret = ff_filter_frame_framed(link, pbuf);
> -}
> if (ret < 0) {
> +if (!link->frame_requested) {
> +av_assert0(ret == AVERROR_EOF);
> +ret = 0;
> +}
> link->frame_requested = 0;
> -if (ret == AVERROR_EOF)
> -link->closed = 1;
> +if (ret == AVERROR_EOF) {
> +ret = ff_filter_link_close(link, AV_NOPTS_VALUE);
> +return ret < 0 ? ret : AVERROR_EOF;
> +}
> } else {
> av_assert0(!link->frame_requested ||
> link->flags & FF_LINK_FLAG_REQUEST_LOOP);
> @@ -1132,10 +1133,52 @@ static int ff_filter_frame_needs_framing(AVFilterLink
> *link, AVFrame *frame)
> return ret;
> }
>
> +static int ff_filter_metaframe(AVFilterLink *link, AVFrame *frame)
> +{
> +AVFrame *pbuf = link->partial_buf;
> +int ret;
> +
> +if (pbuf) {
> +link->partial_buf = NULL;
> +if ((ret = ff_filter_frame_framed(link, pbuf)) < 0)
> +return ret;
> +}
> +
> +if ((link->dst->filter->flags & FF_FILTER_FLAG_SUPPORT_METAFRAMES)) {
> +ret = link->dstpad->filter_frame ?
> + link->dstpad->filter_frame(link, frame) :
> + default_filter_frame(link, frame);
> +if (ret < 0)
> +return ret;
It shall probably execute the remaining part of the function even in
case of failure.
> +}
> +
> +switch (frame->format) {
> +
> +case FF_METAFRAME_EOF:
> +link->closed = 1;
> +break;
> +
> +case 0:
> +case FF_METAFRAME_NOP:
> +/* Not implemented yet because not used either for now.
> + Caveat: if the same metaframe is forwarded to the next filter
> + and the next filter changes the type, the type change must not be
> + taken into account for the first link. */
I find this a bit confusing. Can you explain why the NOP metaframe is
needed?
> +
> +default:
> +av_assert0(!"reached");
> +}
> +
> +return ret;
> +}
> +
> int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
> {
> FF_TPRINTF_START(NULL, filter_frame); ff_tlog_link(NULL, link, 1);
> ff_tlog(NULL, " "); ff_tlog_ref(NULL, frame, 1);
>
> +if (frame->format < -1)
> +return ff_filter_metaframe(link, frame);
> +
> /* Consistency checks */
> if (link->type == AVMEDIA_TYPE_VIDEO) {
> if (strcmp(link->dst->filter->name, "scale")) {
> @@ -1162,6 +1205,22 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
> }
> }
>
> +int ff_filter_link_close(AVFilterLink *link, int64_t pts)
> +{
> +AVFrame *frame;
> +int ret;
> +
> +if (link->closed)
> +return 0;
> +if (!(frame = av_frame_alloc()))
> +return AVERROR(ENOMEM);
> +frame->format = FF_METAFRAME_EOF;
> +frame->pts = pts;
> +ret = ff_filter_frame(link, frame);
> +av_frame_free(&frame);
> +return ret;
> +}
> +
> const AVClass *avfilter_get_class(void)
> {
> return &avfilter_class;
> diff --git a/libavfilter/internal.h b/libavfilter/internal.h
> index 308b115..fbe603a 100644
> --- a/libavfilter/internal.h
> +++ b/libavfilter/internal.h
> @@ -374,4 +374,38 @@ AVFilterContext *ff_filter_alloc(const AVFilter *filter,
> const char *inst_name);
> */
> void ff_filter_graph_remove_filter(AVFilterGraph *graph, AVFilterContext
> *filter);
>
> +/**
> + * The filter can accept metaframes.
> + * Metaframes are AVFrame structures with a negative format field.
> + * The framework will take
Re: [FFmpeg-devel] [PATCH 1/4] lavfi: add filter metaframes infrastructure.
On Wed, Jul 30, 2014 at 11:44:46PM +0200, Nicolas George wrote:
> Metaframes are frames without data, identified by a negative
> format code, used to carry special conditions.
> They are sent only to filter that declare supporting them.
> The only metaframe for now is EOF; this mechanism augments
> the current mechanism based on request_frame() returning
> AVERROR_EOF, with the advantage that the EOF metaframe carries
> a timestamp.
> The metaframes are a purely internal API and do not leak to
> the application.
>
> Signed-off-by: Nicolas George
> ---
> libavfilter/avfilter.c | 73
> +-
> libavfilter/internal.h | 34 +++
> 2 files changed, 100 insertions(+), 7 deletions(-)
>
>
> Changed the name to "metaframes".
> Allocate the frame with the classic method.
>
>
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 7b11467..7894173 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -346,15 +346,16 @@ int ff_request_frame(AVFilterLink *link)
> ret = link->srcpad->request_frame(link);
> else if (link->src->inputs[0])
> ret = ff_request_frame(link->src->inputs[0]);
> -if (ret == AVERROR_EOF && link->partial_buf) {
> -AVFrame *pbuf = link->partial_buf;
> -link->partial_buf = NULL;
> -ret = ff_filter_frame_framed(link, pbuf);
> -}
> if (ret < 0) {
> +if (!link->frame_requested) {
> +av_assert0(ret == AVERROR_EOF);
> +ret = 0;
> +}
> link->frame_requested = 0;
> -if (ret == AVERROR_EOF)
> -link->closed = 1;
> +if (ret == AVERROR_EOF) {
> +ret = ff_filter_link_close(link, AV_NOPTS_VALUE);
> +return ret < 0 ? ret : AVERROR_EOF;
> +}
> } else {
> av_assert0(!link->frame_requested ||
> link->flags & FF_LINK_FLAG_REQUEST_LOOP);
> @@ -1132,10 +1133,52 @@ static int ff_filter_frame_needs_framing(AVFilterLink
> *link, AVFrame *frame)
> return ret;
> }
>
> +static int ff_filter_metaframe(AVFilterLink *link, AVFrame *frame)
> +{
> +AVFrame *pbuf = link->partial_buf;
> +int ret;
ret needs to be initialized to 0 here
(this was what caused tha swr failures i belive)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/4] lavfi: add filter metaframes infrastructure.
Metaframes are frames without data, identified by a negative
format code, used to carry special conditions.
They are sent only to filter that declare supporting them.
The only metaframe for now is EOF; this mechanism augments
the current mechanism based on request_frame() returning
AVERROR_EOF, with the advantage that the EOF metaframe carries
a timestamp.
The metaframes are a purely internal API and do not leak to
the application.
Signed-off-by: Nicolas George
---
libavfilter/avfilter.c | 73 +-
libavfilter/internal.h | 34 +++
2 files changed, 100 insertions(+), 7 deletions(-)
Changed the name to "metaframes".
Allocate the frame with the classic method.
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 7b11467..7894173 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -346,15 +346,16 @@ int ff_request_frame(AVFilterLink *link)
ret = link->srcpad->request_frame(link);
else if (link->src->inputs[0])
ret = ff_request_frame(link->src->inputs[0]);
-if (ret == AVERROR_EOF && link->partial_buf) {
-AVFrame *pbuf = link->partial_buf;
-link->partial_buf = NULL;
-ret = ff_filter_frame_framed(link, pbuf);
-}
if (ret < 0) {
+if (!link->frame_requested) {
+av_assert0(ret == AVERROR_EOF);
+ret = 0;
+}
link->frame_requested = 0;
-if (ret == AVERROR_EOF)
-link->closed = 1;
+if (ret == AVERROR_EOF) {
+ret = ff_filter_link_close(link, AV_NOPTS_VALUE);
+return ret < 0 ? ret : AVERROR_EOF;
+}
} else {
av_assert0(!link->frame_requested ||
link->flags & FF_LINK_FLAG_REQUEST_LOOP);
@@ -1132,10 +1133,52 @@ static int ff_filter_frame_needs_framing(AVFilterLink
*link, AVFrame *frame)
return ret;
}
+static int ff_filter_metaframe(AVFilterLink *link, AVFrame *frame)
+{
+AVFrame *pbuf = link->partial_buf;
+int ret;
+
+if (pbuf) {
+link->partial_buf = NULL;
+if ((ret = ff_filter_frame_framed(link, pbuf)) < 0)
+return ret;
+}
+
+if ((link->dst->filter->flags & FF_FILTER_FLAG_SUPPORT_METAFRAMES)) {
+ret = link->dstpad->filter_frame ?
+ link->dstpad->filter_frame(link, frame) :
+ default_filter_frame(link, frame);
+if (ret < 0)
+return ret;
+}
+
+switch (frame->format) {
+
+case FF_METAFRAME_EOF:
+link->closed = 1;
+break;
+
+case 0:
+case FF_METAFRAME_NOP:
+/* Not implemented yet because not used either for now.
+ Caveat: if the same metaframe is forwarded to the next filter
+ and the next filter changes the type, the type change must not be
+ taken into account for the first link. */
+
+default:
+av_assert0(!"reached");
+}
+
+return ret;
+}
+
int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
{
FF_TPRINTF_START(NULL, filter_frame); ff_tlog_link(NULL, link, 1);
ff_tlog(NULL, " "); ff_tlog_ref(NULL, frame, 1);
+if (frame->format < -1)
+return ff_filter_metaframe(link, frame);
+
/* Consistency checks */
if (link->type == AVMEDIA_TYPE_VIDEO) {
if (strcmp(link->dst->filter->name, "scale")) {
@@ -1162,6 +1205,22 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
}
}
+int ff_filter_link_close(AVFilterLink *link, int64_t pts)
+{
+AVFrame *frame;
+int ret;
+
+if (link->closed)
+return 0;
+if (!(frame = av_frame_alloc()))
+return AVERROR(ENOMEM);
+frame->format = FF_METAFRAME_EOF;
+frame->pts = pts;
+ret = ff_filter_frame(link, frame);
+av_frame_free(&frame);
+return ret;
+}
+
const AVClass *avfilter_get_class(void)
{
return &avfilter_class;
diff --git a/libavfilter/internal.h b/libavfilter/internal.h
index 308b115..fbe603a 100644
--- a/libavfilter/internal.h
+++ b/libavfilter/internal.h
@@ -374,4 +374,38 @@ AVFilterContext *ff_filter_alloc(const AVFilter *filter,
const char *inst_name);
*/
void ff_filter_graph_remove_filter(AVFilterGraph *graph, AVFilterContext
*filter);
+/**
+ * The filter can accept metaframes.
+ * Metaframes are AVFrame structures with a negative format field.
+ * The framework will take default actions based on the metaframe type.
+ * The destination filter is allowed to reset the type to inhibit the
+ * default actions.
+ */
+#define FF_FILTER_FLAG_SUPPORT_METAFRAMES (1 << 24)
+
+/**
+ * Types of metaframes that can be passer to a filter.
+ */
+enum {
+/**
+ * Do not do anything.
+ * Can be used by the destination filter to inhibit default handling.
+ */
+FF_METAFRAME_NOP = -1,
+
+/**
+ * The input has reached EOF.
+ * The pts field holds the timestamp of the end of t
