[FFmpeg-devel,v16,06/16] avfilter/sbuffer: Add sbuffersrc and sbuffersink filters

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 configure                |  2 +-
 libavfilter/allfilters.c |  2 ++
 libavfilter/buffersink.c | 63 +++++++++++++++++++++++++++++++++++
 libavfilter/buffersink.h | 15 +++++++++
 libavfilter/buffersrc.c  | 72 ++++++++++++++++++++++++++++++++++++++++
 libavfilter/buffersrc.h  |  1 +
 6 files changed, 154 insertions(+), 1 deletion(-)

Soft Works:
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  configure                |  2 +-
>  libavfilter/allfilters.c |  2 ++
>  libavfilter/buffersink.c | 63 +++++++++++++++++++++++++++++++++++
>  libavfilter/buffersink.h | 15 +++++++++
>  libavfilter/buffersrc.c  | 72 ++++++++++++++++++++++++++++++++++++++++
>  libavfilter/buffersrc.h  |  1 +
>  6 files changed, 154 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index d068b11073..e4d1443237 100755
> --- a/configure
> +++ b/configure
> @@ -7758,7 +7758,7 @@ print_enabled_components(){
>          fi
>      done
>      if [ "$name" = "filter_list" ]; then
> -        for c in asrc_abuffer vsrc_buffer asink_abuffer vsink_buffer; do
> +        for c in asrc_abuffer vsrc_buffer ssrc_sbuffer asink_abuffer vsink_buffer ssink_sbuffer; do
>              printf "    &ff_%s,\n" $c >> $TMPH
>          done
>      fi
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 4bf17ef292..4072f08385 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -550,8 +550,10 @@ extern const AVFilter ff_avsrc_movie;
>   * being the same while having different 'types'). */
>  extern  const AVFilter ff_asrc_abuffer;
>  extern  const AVFilter ff_vsrc_buffer;
> +extern  const AVFilter ff_ssrc_sbuffer;
>  extern  const AVFilter ff_asink_abuffer;
>  extern  const AVFilter ff_vsink_buffer;
> +extern  const AVFilter ff_ssink_sbuffer;
>  extern const AVFilter ff_af_afifo;
>  extern const AVFilter ff_vf_fifo;
>  
> diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c
> index b8ddafec35..8306312acc 100644
> --- a/libavfilter/buffersink.c
> +++ b/libavfilter/buffersink.c
> @@ -29,6 +29,8 @@
>  #include "libavutil/internal.h"
>  #include "libavutil/opt.h"
>  
> +#include "libavcodec/avcodec.h"
> +
>  #define FF_INTERNAL_FIELDS 1
>  #include "framequeue.h"
>  
> @@ -57,6 +59,10 @@ typedef struct BufferSinkContext {
>      int *sample_rates;                      ///< list of accepted sample rates, terminated by -1
>      int sample_rates_size;
>  
> +    /* only used for subtitles */
> +    enum AVSubtitleType *subtitle_types;     ///< list of accepted subtitle types, must be terminated with -1

subtitle_types is not terminated by -1 at all; it uses the size field
below instead of a sentinel. The same is true for the other arrays.

> +    int subtitle_types_size;
> +
>      AVFrame *peeked_frame;
>  } BufferSinkContext;
>  
> @@ -168,6 +174,15 @@ AVABufferSinkParams *av_abuffersink_params_alloc(void)
>          return NULL;
>      return params;
>  }
> +
> +AVSBufferSinkParams *av_sbuffersink_params_alloc(void)
> +{
> +    AVSBufferSinkParams *params = av_mallocz(sizeof(AVSBufferSinkParams));
> +
> +    if (!params)
> +        return NULL;
> +    return params;
> +}
>  #endif
>  
>  static av_cold int common_init(AVFilterContext *ctx)
> @@ -305,6 +320,28 @@ static int asink_query_formats(AVFilterContext *ctx)
>      return 0;
>  }
>  
> +static int ssink_query_formats(AVFilterContext *ctx)
> +{
> +    BufferSinkContext *buf = ctx->priv;
> +    AVFilterFormats *formats = NULL;
> +    unsigned i;
> +    int ret;
> +
> +    CHECK_LIST_SIZE(subtitle_types)
> +    if (buf->subtitle_types_size) {
> +        for (i = 0; i < NB_ITEMS(buf->subtitle_types); i++)
> +            if ((ret = ff_add_subtitle_type(&formats, buf->subtitle_types[i])) < 0)
> +                return ret;
> +        if ((ret = ff_set_common_formats(ctx, formats)) < 0)
> +            return ret;
> +    } else {
> +        if ((ret = ff_default_query_formats(ctx)) < 0)
> +            return ret;
> +    }
> +
> +    return 0;
> +}
> +
>  #define OFFSET(x) offsetof(BufferSinkContext, x)
>  #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
>  static const AVOption buffersink_options[] = {
> @@ -322,9 +359,16 @@ static const AVOption abuffersink_options[] = {
>      { NULL },
>  };
>  #undef FLAGS
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_SUBTITLE_PARAM
> +static const AVOption sbuffersink_options[] = {
> +    { "subtitle_types", "set the supported subtitle formats", OFFSET(subtitle_types), AV_OPT_TYPE_BINARY, .flags = FLAGS },
> +    { NULL },
> +};
> +#undef FLAGS
>  
>  AVFILTER_DEFINE_CLASS(buffersink);
>  AVFILTER_DEFINE_CLASS(abuffersink);
> +AVFILTER_DEFINE_CLASS(sbuffersink);
>  
>  static const AVFilterPad avfilter_vsink_buffer_inputs[] = {
>      {
> @@ -363,3 +407,22 @@ const AVFilter ff_asink_abuffer = {
>      .outputs       = NULL,
>      FILTER_QUERY_FUNC(asink_query_formats),
>  };
> +
> +static const AVFilterPad avfilter_ssink_sbuffer_inputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_SUBTITLE,
> +    },
> +};
> +
> +const AVFilter ff_ssink_sbuffer = {
> +    .name          = "sbuffersink",
> +    .description   = NULL_IF_CONFIG_SMALL("Buffer subtitle frames, and make them available to the end of the filter graph."),
> +    .priv_class    = &sbuffersink_class,
> +    .priv_size     = sizeof(BufferSinkContext),
> +    .init          = common_init,
> +    .activate      = activate,
> +    FILTER_INPUTS(avfilter_ssink_sbuffer_inputs),
> +    .outputs       = NULL,
> +    FILTER_QUERY_FUNC(ssink_query_formats),
> +};
> diff --git a/libavfilter/buffersink.h b/libavfilter/buffersink.h
> index 69ed0f29a8..b439b586c5 100644
> --- a/libavfilter/buffersink.h
> +++ b/libavfilter/buffersink.h
> @@ -129,6 +129,21 @@ typedef struct AVABufferSinkParams {
>   */
>  attribute_deprecated
>  AVABufferSinkParams *av_abuffersink_params_alloc(void);
> +
> +/**
> + * Deprecated and unused struct to use for initializing an sbuffersink context.
> + */
> +typedef struct AVSBufferSinkParams {
> +    const int *subtitle_type;
> +} AVSBufferSinkParams;
> +
> +/**
> + * Create an AVSBufferSinkParams structure.
> + *
> + * Must be freed with av_free().
> + */
> +attribute_deprecated
> +AVSBufferSinkParams *av_sbuffersink_params_alloc(void);
>  #endif

Just don't add it.

>  
>  /**
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index b0611872f1..8f22587f42 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -39,6 +39,7 @@
>  #include "formats.h"
>  #include "internal.h"
>  #include "video.h"
> +#include "libavcodec/avcodec.h"
>  
>  typedef struct BufferSourceContext {
>      const AVClass    *class;
> @@ -63,6 +64,9 @@ typedef struct BufferSourceContext {
>      uint64_t channel_layout;
>      char    *channel_layout_str;
>  
> +    /* subtitle only */
> +    enum AVSubtitleType subtitle_type;
> +
>      int eof;
>  } BufferSourceContext;
>  
> @@ -130,6 +134,13 @@ int av_buffersrc_parameters_set(AVFilterContext *ctx, AVBufferSrcParameters *par
>          if (param->channel_layout)
>              s->channel_layout = param->channel_layout;
>          break;
> +    case AVMEDIA_TYPE_SUBTITLE:
> +        s->subtitle_type = param->format;
> +        if (param->width > 0)
> +            s->w = param->width;
> +        if (param->height > 0)
> +            s->h = param->height;
> +        break;
>      default:
>          return AVERROR_BUG;
>      }
> @@ -197,6 +208,8 @@ int attribute_align_arg av_buffersrc_add_frame_flags(AVFilterContext *ctx, AVFra
>              CHECK_AUDIO_PARAM_CHANGE(ctx, s, frame->sample_rate, frame->channel_layout,
>                                       frame->channels, frame->format, frame->pts);
>              break;
> +        case AVMEDIA_TYPE_SUBTITLE:
> +            break;
>          default:
>              return AVERROR(EINVAL);
>          }
> @@ -269,6 +282,7 @@ unsigned av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src)
>  #define OFFSET(x) offsetof(BufferSourceContext, x)
>  #define A AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_AUDIO_PARAM
>  #define V AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +#define S AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_SUBTITLE_PARAM
>  
>  static const AVOption buffer_options[] = {
>      { "width",         NULL,                     OFFSET(w),                AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, V },
> @@ -298,6 +312,16 @@ static const AVOption abuffer_options[] = {
>  
>  AVFILTER_DEFINE_CLASS(abuffer);
>  
> +static const AVOption sbuffer_options[] = {
> +    { "time_base",     NULL, OFFSET(time_base),            AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, INT_MAX, S },
> +    { "subtitle_type", NULL, OFFSET(subtitle_type),        AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, S },
> +    { "width",         NULL, OFFSET(w),                    AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, V },
> +    { "height",        NULL, OFFSET(h),                    AV_OPT_TYPE_INT,      { .i64 = 0 }, 0, INT_MAX, V },
> +    { NULL },
> +};
> +
> +AVFILTER_DEFINE_CLASS(sbuffer);
> +
>  static av_cold int init_audio(AVFilterContext *ctx)
>  {
>      BufferSourceContext *s = ctx->priv;
> @@ -347,6 +371,21 @@ static av_cold int init_audio(AVFilterContext *ctx)
>      return ret;
>  }
>  
> +static av_cold int init_subtitle(AVFilterContext *ctx)
> +{
> +    BufferSourceContext *c = ctx->priv;
> +
> +    if (c->subtitle_type == AV_SUBTITLE_FMT_BITMAP)
> +        av_log(ctx, AV_LOG_VERBOSE, "graphical subtitles - w:%d h:%d tb:%d/%d\n",
> +               c->w, c->h, c->time_base.num, c->time_base.den);
> +    else
> +        av_log(ctx, AV_LOG_VERBOSE, "text subtitles -  w:%d h:%d tb:%d/%d\n",
> +               c->w, c->h, c->time_base.num, c->time_base.den);
> +
> +    return 0;
> +}
> +
> +
>  static av_cold void uninit(AVFilterContext *ctx)
>  {
>      BufferSourceContext *s = ctx->priv;
> @@ -381,6 +420,11 @@ static int query_formats(AVFilterContext *ctx)
>          if ((ret = ff_set_common_channel_layouts(ctx, channel_layouts)) < 0)
>              return ret;
>          break;
> +    case AVMEDIA_TYPE_SUBTITLE:
> +        if ((ret = ff_add_format         (&formats, c->subtitle_type)) < 0 ||
> +            (ret = ff_set_common_formats (ctx     , formats   )) < 0)
> +            return ret;
> +        break;
>      default:
>          return AVERROR(EINVAL);
>      }
> @@ -408,6 +452,11 @@ static int config_props(AVFilterLink *link)
>          if (!c->channel_layout)
>              c->channel_layout = link->channel_layout;
>          break;
> +    case AVMEDIA_TYPE_SUBTITLE:
> +        link->format = c->subtitle_type;
> +        link->w = c->w;
> +        link->h = c->h;
> +        break;
>      default:
>          return AVERROR(EINVAL);
>      }
> @@ -472,3 +521,26 @@ const AVFilter ff_asrc_abuffer = {
>      FILTER_QUERY_FUNC(query_formats),
>      .priv_class = &abuffer_class,
>  };
> +
> +static const AVFilterPad avfilter_ssrc_sbuffer_outputs[] = {

Can you avoid the avfilter prefix (which violates our typical naming
conventions notwithstanding then fact that other parts of this file also
do it)?

> +    {
> +        .name          = "default",
> +        .type          = AVMEDIA_TYPE_SUBTITLE,
> +        .request_frame = request_frame,
> +        .config_props  = config_props,
> +    },
> +};
> +
> +const AVFilter ff_ssrc_sbuffer = {
> +    .name          = "sbuffer",
> +    .description   = NULL_IF_CONFIG_SMALL("Buffer subtitle frames, and make them accessible to the filterchain."),
> +    .priv_size     = sizeof(BufferSourceContext),
> +
> +    .init      = init_subtitle,
> +    .uninit    = uninit,
> +
> +    .inputs    = NULL,
> +    FILTER_OUTPUTS(avfilter_ssrc_sbuffer_outputs),
> +    FILTER_QUERY_FUNC(query_formats),
> +    .priv_class = &sbuffer_class,
> +};
> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
> index 08fbd18a47..929a2fa249 100644
> --- a/libavfilter/buffersrc.h
> +++ b/libavfilter/buffersrc.h
> @@ -74,6 +74,7 @@ typedef struct AVBufferSrcParameters {
>      /**
>       * video: the pixel format, value corresponds to enum AVPixelFormat
>       * audio: the sample format, value corresponds to enum AVSampleFormat
> +     * subtitles: the subtitle format, value corresponds to enum AVSubtitleType
>       */
>      int format;
>      /**
>

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Andreas
> Rheinhardt
> Sent: Friday, November 26, 2021 12:14 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v16 06/16] avfilter/sbuffer: Add
> sbuffersrc and sbuffersink filters
> 
> > +#include "libavcodec/avcodec.h"
> > +
> >  #define FF_INTERNAL_FIELDS 1
> >  #include "framequeue.h"
> >
> > @@ -57,6 +59,10 @@ typedef struct BufferSinkContext {
> >      int *sample_rates;                      ///< list of accepted sample
> rates, terminated by -1
> >      int sample_rates_size;
> >
> > +    /* only used for subtitles */
> > +    enum AVSubtitleType *subtitle_types;     ///< list of accepted
> subtitle types, must be terminated with -1
> 
> subtitle_types is not terminated by -1 at all; it uses the size field
> below instead of a sentinel. The same is true for the other arrays.

My patch does things just analog to the video and audio implementation.
I have no idea in which direction this is intended to go and I think it
is outside the scope of this patch to make any assumptions about
it. Even when it would be clear in which way this should be changed in the 
future - I don't think such change should be part of this patch. 
Also whatever I'd do, I'm sure it would take multiple additional patch 
revisions until everybody is satisfied.
This can be changed later (consistently for all three media types at once),
or - if you see some urgency in this - please change it in the main tree, 
then I'll adapt my patch. 

All other suggestions done.

Thanks,
sw