diff mbox series

[FFmpeg-devel,2/9] avfilter/subtitles: Add subtitles.c

Message ID MN2PR04MB5981DBDB4F760B4793BC11E3BAC09@MN2PR04MB5981.namprd04.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel,1/9] lavu/frame: avframe add type property
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Soft Works Aug. 19, 2021, 7:43 a.m. UTC
Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavfilter/Makefile    |  1 +
 libavfilter/internal.h  |  8 ++++++
 libavfilter/subtitles.c | 61 +++++++++++++++++++++++++++++++++++++++++
 libavfilter/subtitles.h | 42 ++++++++++++++++++++++++++++
 4 files changed, 112 insertions(+)
 create mode 100644 libavfilter/subtitles.c
 create mode 100644 libavfilter/subtitles.h

Comments

Nicolas George Aug. 19, 2021, 8:29 a.m. UTC | #1
Soft Works (12021-08-19):
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavfilter/Makefile    |  1 +
>  libavfilter/internal.h  |  8 ++++++
>  libavfilter/subtitles.c | 61 +++++++++++++++++++++++++++++++++++++++++
>  libavfilter/subtitles.h | 42 ++++++++++++++++++++++++++++
>  4 files changed, 112 insertions(+)
>  create mode 100644 libavfilter/subtitles.c
>  create mode 100644 libavfilter/subtitles.h

The change in internal.h is conflicting with Andrea's work.

Regards,
Soft Works Aug. 19, 2021, 8:57 a.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Thursday, 19 August 2021 10:30
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/9] avfilter/subtitles: Add subtitles.c
> 
> Soft Works (12021-08-19):
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >  libavfilter/Makefile    |  1 +
> >  libavfilter/internal.h  |  8 ++++++
> >  libavfilter/subtitles.c | 61 +++++++++++++++++++++++++++++++++++++++++
> >  libavfilter/subtitles.h | 42 ++++++++++++++++++++++++++++
> >  4 files changed, 112 insertions(+)
> >  create mode 100644 libavfilter/subtitles.c
> >  create mode 100644 libavfilter/subtitles.h
> 
> The change in internal.h is conflicting with Andrea's work.

You mean Andreas' work? Committed or uncommitted?

Regarding internal.h, my patch is on top of

"avfilter/avfilter: Remove unused feature to add pads in the middle"
515e7fbce1cc555d9eaadad798d3d968ff468987
Nicolas George Aug. 19, 2021, 9:07 a.m. UTC | #3
Soft Works (12021-08-19):
> You mean Andreas' work? Committed or uncommitted?

https://ffmpeg.org/pipermail/ffmpeg-devel/2021-August/283930.html

Approved but not yet committed.

Regards,
Soft Works Aug. 19, 2021, 9:30 a.m. UTC | #4
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Thursday, 19 August 2021 11:08
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/9] avfilter/subtitles: Add subtitles.c
> 
> Soft Works (12021-08-19):
> > You mean Andreas' work? Committed or uncommitted?
> 
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-August/283930.html
> 
> Approved but not yet committed.

I can't update my patch before it's committed at it would break compilation,
but thanks a lot for the hint!

softworkz
Nicolas George Aug. 19, 2021, 9:31 a.m. UTC | #5
Soft Works (12021-08-19):
> I can't update my patch before it's committed

You can, you just need to apply Andrea's patch on your tree.
Soft Works Aug. 19, 2021, 9:33 a.m. UTC | #6
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Thursday, 19 August 2021 11:32
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/9] avfilter/subtitles: Add subtitles.c
> 
> Soft Works (12021-08-19):
> > I can't update my patch before it's committed
> 
> You can, you just need to apply Andrea's patch on your tree.
> 

And patchwork running Fate?
Nicolas George Aug. 19, 2021, 9:34 a.m. UTC | #7
Soft Works (12021-08-19):
> And patchwork running Fate?

You do not need to care.

I pretty much hope you run FATE, including samples, yourself before
sending any patch.
Soft Works Aug. 19, 2021, 9:43 a.m. UTC | #8
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Thursday, 19 August 2021 11:35
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/9] avfilter/subtitles: Add subtitles.c
> 
> Soft Works (12021-08-19):
> > And patchwork running Fate?
> 
> You do not need to care.
> 
> I pretty much hope you run FATE, including samples, yourself before
> sending any patch.

Yes I did, but I had seen some weird effects:

I got an error from checkasm av_tx when I did a major version bump
for libavutil.

Also, tests/data/filtergraphs/colorkey failed locally for some reason
and the 'eval' test had a different result for 

Evaluating 'not(NAN)'
'not(NAN)' -> 0.000000
(but I got 'not(NAN)' -> 1.000000)

Finally, I don't want to see red "bubbles" for my patches on patchwork ;-)

softworkz
Nicolas George Aug. 19, 2021, 1:04 p.m. UTC | #9
Soft Works (12021-08-19):
> I got an error from checkasm av_tx when I did a major version bump
> for libavutil.

You are not to bump the major version on your own, it is decision taken
collectively time in advance.

> Also, tests/data/filtergraphs/colorkey failed locally for some reason
> and the 'eval' test had a different result for 
> 
> Evaluating 'not(NAN)'
> 'not(NAN)' -> 0.000000
> (but I got 'not(NAN)' -> 1.000000)

This is suspicious.

Regards,
Soft Works Aug. 19, 2021, 10:15 p.m. UTC | #10
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Thursday, 19 August 2021 15:05
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/9] avfilter/subtitles: Add subtitles.c
> 
> Soft Works (12021-08-19):
> > I got an error from checkasm av_tx when I did a major version bump
> > for libavutil.
> 
> You are not to bump the major version on your own, it is decision taken
> collectively time in advance.

OK, understood.

> 
> > Also, tests/data/filtergraphs/colorkey failed locally for some reason
> > and the 'eval' test had a different result for
> >
> > Evaluating 'not(NAN)'
> > 'not(NAN)' -> 0.000000
> > (but I got 'not(NAN)' -> 1.000000)
> 
> This is suspicious.

I'm not sure whether it might be due to running under Windows/MSYS2..?

softworkz
Andreas Rheinhardt Aug. 20, 2021, 2:50 a.m. UTC | #11
Nicolas George:
> Soft Works (12021-08-19):
>> You mean Andreas' work? Committed or uncommitted?
> 
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-August/283930.html
> 
> Approved but not yet committed.
> 
That's wrong: It has already been committed as
1aa640c7d785c39e0e19407521132d77b594e654.
(You can already see the union in his diff.)

- Andreas
Andreas Rheinhardt Aug. 20, 2021, 3:10 a.m. UTC | #12
Soft Works:
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavfilter/Makefile    |  1 +
>  libavfilter/internal.h  |  8 ++++++
>  libavfilter/subtitles.c | 61 +++++++++++++++++++++++++++++++++++++++++
>  libavfilter/subtitles.h | 42 ++++++++++++++++++++++++++++
>  4 files changed, 112 insertions(+)
>  create mode 100644 libavfilter/subtitles.c
>  create mode 100644 libavfilter/subtitles.h
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 102ce7beff..db32cf1265 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -19,6 +19,7 @@ OBJS = allfilters.o                                                     \
>         framequeue.o                                                     \
>         graphdump.o                                                      \
>         graphparser.o                                                    \
> +       subtitles.o                                                          \

Weird alignment.

>         video.o                                                          \
>  
>  OBJS-$(HAVE_THREADS)                         += pthread.o
> diff --git a/libavfilter/internal.h b/libavfilter/internal.h
> index a0aa32af4d..4f9aedf151 100644
> --- a/libavfilter/internal.h
> +++ b/libavfilter/internal.h
> @@ -87,6 +87,14 @@ struct AVFilterPad {
>          AVFrame *(*audio)(AVFilterLink *link, int nb_samples);
>      } get_buffer;
>  
> +    /**
> +     * Callback function to get a subtitle buffer. If NULL, the filter system will
> +     * use ff_default_get_subtitle_buffer().
> +     *
> +     * Input subtitle pads only.
> +     */
> +    AVFrame *(*get_subtitle_buffer)(AVFilterLink *link, int format);
> +

While there is no git-conflict with my patch, there is a conflict in
spirit: This pad has one type and only needs one get_buffer callback. In
other words, put it in the union above.

>      /**
>       * Filtering callback. This is where a filter receives a frame with
>       * audio/video data and should do its processing.
> diff --git a/libavfilter/subtitles.c b/libavfilter/subtitles.c
> new file mode 100644
> index 0000000000..188ff1289c
> --- /dev/null
> +++ b/libavfilter/subtitles.c
> @@ -0,0 +1,61 @@
> +/*
> + * Copyright (c) 2021 softworkz
> + *
> + * 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
> + */
> +
> +#include "libavutil/common.h"
> +
> +#include "subtitles.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +
> +AVFrame *ff_null_get_subtitles_buffer(AVFilterLink *link, int format)
> +{
> +    return ff_get_subtitles_buffer(link->dst->outputs[0], format);
> +}
> +
> +AVFrame *ff_default_get_subtitles_buffer(AVFilterLink *link, int format)
> +{
> +    AVFrame *frame = NULL;
> +
> +    // TODO:
> +    //frame = ff_frame_pool_get(link->frame_pool);
> +
> +    frame = av_frame_alloc();
> +    if (!frame)
> +        return NULL;
> +
> +    frame->format = format;
> +    frame->type = AVMEDIA_TYPE_SUBTITLE;
> +
> +    return frame;
> +}
> +
> +AVFrame *ff_get_subtitles_buffer(AVFilterLink *link, int format)
> +{
> +    AVFrame *ret = NULL;
> +
> +    if (link->dstpad->get_subtitle_buffer)
> +        ret = link->dstpad->get_subtitle_buffer(link, format);
> +
> +    if (!ret)
> +        ret = ff_default_get_subtitles_buffer(link, format);
> +
> +    return ret;
> +}
> diff --git a/libavfilter/subtitles.h b/libavfilter/subtitles.h
> new file mode 100644
> index 0000000000..b964b61c37
> --- /dev/null
> +++ b/libavfilter/subtitles.h
> @@ -0,0 +1,42 @@
> +/*
> + * Copyright (c) 2021 softworkz
> + *
> + * 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_SUBTITLES_H
> +#define AVFILTER_SUBTITLES_H
> +
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +/** default handler for get_subtitles_buffer() for subtitle inputs */
> +AVFrame *ff_default_get_subtitles_buffer(AVFilterLink *link, int format);
> +
> +/** get_subtitles_buffer() handler for filters which simply pass subtitles along */
> +AVFrame *ff_null_get_subtitles_buffer(AVFilterLink *link, int format);
> +
> +/**
> + * Request a subtitles buffer with a specific set of permissions.
> + *
> + * @param link           the output link to the filter from which the buffer will
> + *                       be requested
> + * @return               The subtitles format.

I suppose this is supposed to be the documentation for format.

> + */
> +AVFrame *ff_get_subtitles_buffer(AVFilterLink *link, int format);
> +
> +#endif /* AVFILTER_SUBTITLES_H */
>
diff mbox series

Patch

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 102ce7beff..db32cf1265 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -19,6 +19,7 @@  OBJS = allfilters.o                                                     \
        framequeue.o                                                     \
        graphdump.o                                                      \
        graphparser.o                                                    \
+       subtitles.o                                                          \
        video.o                                                          \
 
 OBJS-$(HAVE_THREADS)                         += pthread.o
diff --git a/libavfilter/internal.h b/libavfilter/internal.h
index a0aa32af4d..4f9aedf151 100644
--- a/libavfilter/internal.h
+++ b/libavfilter/internal.h
@@ -87,6 +87,14 @@  struct AVFilterPad {
         AVFrame *(*audio)(AVFilterLink *link, int nb_samples);
     } get_buffer;
 
+    /**
+     * Callback function to get a subtitle buffer. If NULL, the filter system will
+     * use ff_default_get_subtitle_buffer().
+     *
+     * Input subtitle pads only.
+     */
+    AVFrame *(*get_subtitle_buffer)(AVFilterLink *link, int format);
+
     /**
      * Filtering callback. This is where a filter receives a frame with
      * audio/video data and should do its processing.
diff --git a/libavfilter/subtitles.c b/libavfilter/subtitles.c
new file mode 100644
index 0000000000..188ff1289c
--- /dev/null
+++ b/libavfilter/subtitles.c
@@ -0,0 +1,61 @@ 
+/*
+ * Copyright (c) 2021 softworkz
+ *
+ * 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
+ */
+
+#include "libavutil/common.h"
+
+#include "subtitles.h"
+#include "avfilter.h"
+#include "internal.h"
+
+
+AVFrame *ff_null_get_subtitles_buffer(AVFilterLink *link, int format)
+{
+    return ff_get_subtitles_buffer(link->dst->outputs[0], format);
+}
+
+AVFrame *ff_default_get_subtitles_buffer(AVFilterLink *link, int format)
+{
+    AVFrame *frame = NULL;
+
+    // TODO:
+    //frame = ff_frame_pool_get(link->frame_pool);
+
+    frame = av_frame_alloc();
+    if (!frame)
+        return NULL;
+
+    frame->format = format;
+    frame->type = AVMEDIA_TYPE_SUBTITLE;
+
+    return frame;
+}
+
+AVFrame *ff_get_subtitles_buffer(AVFilterLink *link, int format)
+{
+    AVFrame *ret = NULL;
+
+    if (link->dstpad->get_subtitle_buffer)
+        ret = link->dstpad->get_subtitle_buffer(link, format);
+
+    if (!ret)
+        ret = ff_default_get_subtitles_buffer(link, format);
+
+    return ret;
+}
diff --git a/libavfilter/subtitles.h b/libavfilter/subtitles.h
new file mode 100644
index 0000000000..b964b61c37
--- /dev/null
+++ b/libavfilter/subtitles.h
@@ -0,0 +1,42 @@ 
+/*
+ * Copyright (c) 2021 softworkz
+ *
+ * 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_SUBTITLES_H
+#define AVFILTER_SUBTITLES_H
+
+#include "avfilter.h"
+#include "internal.h"
+
+/** default handler for get_subtitles_buffer() for subtitle inputs */
+AVFrame *ff_default_get_subtitles_buffer(AVFilterLink *link, int format);
+
+/** get_subtitles_buffer() handler for filters which simply pass subtitles along */
+AVFrame *ff_null_get_subtitles_buffer(AVFilterLink *link, int format);
+
+/**
+ * Request a subtitles buffer with a specific set of permissions.
+ *
+ * @param link           the output link to the filter from which the buffer will
+ *                       be requested
+ * @return               The subtitles format.
+ */
+AVFrame *ff_get_subtitles_buffer(AVFilterLink *link, int format);
+
+#endif /* AVFILTER_SUBTITLES_H */