[FFmpeg-devel,v18,01/19] avcodec, avutil: Move enum AVSubtitleType

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavcodec/avcodec.h | 19 +--------------
 libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 libavutil/subfmt.h

Quoting Soft Works (2021-11-29 20:47:52)
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavcodec/avcodec.h | 19 +--------------
>  libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+), 18 deletions(-)
>  create mode 100644 libavutil/subfmt.h

This commit does way more than just moving the enum, it also
deprecates existing enum values and adds new ones. That should be
mentioned in the commit message and in doc/APIchanges.

Adding the header to the installed header list should also be moved to
this commit.

The newly-deprecated enum values should be put under removal guards
(i.e. #ifdef FF_API_OLD_SUBTITLES ...)

> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 7ee8bc2b7c..b05c12e47e 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -35,6 +35,7 @@
>  #include "libavutil/frame.h"
>  #include "libavutil/log.h"
>  #include "libavutil/pixfmt.h"
> +#include "libavutil/subfmt.h"
>  #include "libavutil/rational.h"
>  
>  #include "codec.h"
> @@ -2238,24 +2239,6 @@ typedef struct AVHWAccel {
>   * @}
>   */
>  
> -enum AVSubtitleType {
> -    SUBTITLE_NONE,
> -
> -    SUBTITLE_BITMAP,                ///< A bitmap, pict will be set
> -
> -    /**
> -     * Plain text, the text field must be set by the decoder and is
> -     * authoritative. ass and pict fields may contain approximations.
> -     */
> -    SUBTITLE_TEXT,
> -
> -    /**
> -     * Formatted text, the ass field must be set by the decoder and is
> -     * authoritative. pict and text fields may contain approximations.
> -     */
> -    SUBTITLE_ASS,
> -};
> -
>  #define AV_SUBTITLE_FLAG_FORCED 0x00000001
>  
>  typedef struct AVSubtitleRect {
> diff --git a/libavutil/subfmt.h b/libavutil/subfmt.h
> new file mode 100644
> index 0000000000..5fc41d0ef2
> --- /dev/null
> +++ b/libavutil/subfmt.h
> @@ -0,0 +1,58 @@
> +/*
> + * 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 AVUTIL_SUBFMT_H
> +#define AVUTIL_SUBFMT_H
> +
> +enum AVSubtitleType {
> +
> +    /**
> +     * Subtitle format unknown.
> +     */
> +    AV_SUBTITLE_FMT_NONE = -1,
> +
> +    /**
> +     * Subtitle format unknown.
> +     */
> +    AV_SUBTITLE_FMT_UNKNOWN = 0,
> +    SUBTITLE_NONE = 0,          ///< Deprecated, use AV_SUBTITLE_FMT_NONE instead.

This looks suspicious. Are you sure it's not going to break existing
code?

> +
> +    /**
> +     * Bitmap area in AVSubtitleRect.data, pixfmt AV_PIX_FMT_PAL8.
> +     */
> +    AV_SUBTITLE_FMT_BITMAP = 1,
> +    SUBTITLE_BITMAP = 1,        ///< Deprecated, use AV_SUBTITLE_FMT_BITMAP instead.
> +
> +    /**
> +     * Plain text in AVSubtitleRect.text.
> +     */
> +    AV_SUBTITLE_FMT_TEXT = 2,
> +    SUBTITLE_TEXT = 2,          ///< Deprecated, use AV_SUBTITLE_FMT_TEXT instead.
> +
> +    /**
> +     * Text Formatted as per ASS specification, contained AVSubtitleRect.ass.
> +     */
> +    AV_SUBTITLE_FMT_ASS = 3,
> +    SUBTITLE_ASS = 3,           ///< Deprecated, use AV_SUBTITLE_FMT_ASS instead.
> +
> +    AV_SUBTITLE_FMT_NB,

The use of this field should be documented. If it's a part of public
API, you are preventing any new types from being added without an
API/ABI break.

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Wednesday, December 1, 2021 2:35 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> AVSubtitleType
> 
> Quoting Soft Works (2021-11-29 20:47:52)
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >  libavcodec/avcodec.h | 19 +--------------
> >  libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+), 18 deletions(-)
> >  create mode 100644 libavutil/subfmt.h
> 
> This commit does way more than just moving the enum, it also
> deprecates existing enum values and adds new ones. That should be
> mentioned in the commit message and in doc/APIchanges.

Will do.

> Adding the header to the installed header list should also be moved to
> this commit.

Yes of course!


> The newly-deprecated enum values should be put under removal guards
> (i.e. #ifdef FF_API_OLD_SUBTITLES ...)

At which place should FF_API_OLD_SUBTITLES get defined then (set to 1 for now)?


> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 7ee8bc2b7c..b05c12e47e 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -35,6 +35,7 @@
> >  #include "libavutil/frame.h"
> >  #include "libavutil/log.h"
> >  #include "libavutil/pixfmt.h"
> > +#include "libavutil/subfmt.h"
> >  #include "libavutil/rational.h"
> >
> >  #include "codec.h"
> > @@ -2238,24 +2239,6 @@ typedef struct AVHWAccel {
> >   * @}
> >   */
> >
> > -enum AVSubtitleType {
> > -    SUBTITLE_NONE,
> > -
> > -    SUBTITLE_BITMAP,                ///< A bitmap, pict will be set
> > -
> > -    /**
> > -     * Plain text, the text field must be set by the decoder and is
> > -     * authoritative. ass and pict fields may contain approximations.
> > -     */
> > -    SUBTITLE_TEXT,
> > -
> > -    /**
> > -     * Formatted text, the ass field must be set by the decoder and is
> > -     * authoritative. pict and text fields may contain approximations.
> > -     */
> > -    SUBTITLE_ASS,
> > -};
> > -
> >  #define AV_SUBTITLE_FLAG_FORCED 0x00000001
> >
> >  typedef struct AVSubtitleRect {
> > diff --git a/libavutil/subfmt.h b/libavutil/subfmt.h
> > new file mode 100644
> > index 0000000000..5fc41d0ef2
> > --- /dev/null
> > +++ b/libavutil/subfmt.h
> > @@ -0,0 +1,58 @@
> > +/*
> > + * 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 AVUTIL_SUBFMT_H
> > +#define AVUTIL_SUBFMT_H
> > +
> > +enum AVSubtitleType {
> > +
> > +    /**
> > +     * Subtitle format unknown.
> > +     */
> > +    AV_SUBTITLE_FMT_NONE = -1,
> > +
> > +    /**
> > +     * Subtitle format unknown.
> > +     */
> > +    AV_SUBTITLE_FMT_UNKNOWN = 0,
> > +    SUBTITLE_NONE = 0,          ///< Deprecated, use AV_SUBTITLE_FMT_NONE
> instead.
> 
> This looks suspicious. Are you sure it's not going to break existing
> code?

Yes I am. In the legacy API, SUBTITLE_NONE actually had a meaning like 
"unspecified", "unknown" or "not set" (obviously - having a value of int 0).
The ***_NONE values of the regular APIs though (e.g. AV_PIXFMT_NONE) 
have a rather obscure semantic - sometimes being used as array sentinels, 
sometimes for tristate sef/unset logic etc.

SUBTITLE_NONE was nothing like that. It's the logical equivalent to 
AV_SUBTITLE_FMT_UNKNOWN, despite the naming confusion. 

I should also add that the actual define SUBTITLE_NONE has been used at a single
place in all ffmpeg code only (https://github.com/FFmpeg/FFmpeg/search?q=SUBTITLE_NONE).

The naming of the new constants is chosen to follow the API for audio and video
(AV_SAMPLE_FMT_, AV_PIXFMT_). Actually I tried to establish equality to the
other two wherever possible, even when something could have been "simplified",
but I think that uniformity ("know one, know the other") is a much higher value
from an API perspective than saving two or three functions and a few parameters 
here and there.


> > +    /**
> > +     * Text Formatted as per ASS specification, contained
> AVSubtitleRect.ass.
> > +     */
> > +    AV_SUBTITLE_FMT_ASS = 3,
> > +    SUBTITLE_ASS = 3,           ///< Deprecated, use AV_SUBTITLE_FMT_ASS
> instead.
> > +
> > +    AV_SUBTITLE_FMT_NB,
> 
> The use of this field should be documented. If it's a part of public
> API, you are preventing any new types from being added without an
> API/ABI break.

I guess you mean this text?

    AV_PIX_FMT_NB         ///< number of pixel formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions


Thank you very much,
softworkz

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Wednesday, December 1, 2021 2:35 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> AVSubtitleType
> 
> Quoting Soft Works (2021-11-29 20:47:52)
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >  libavcodec/avcodec.h | 19 +--------------
> >  libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+), 18 deletions(-)
> >  create mode 100644 libavutil/subfmt.h
> 
> This commit does way more than just moving the enum, it also
> deprecates existing enum values and adds new ones. That should be
> mentioned in the commit message and in doc/APIchanges.

This is something I would need some help with. Which of my commits should 
bump versions in which libs and where major or minor bump?

Here's what I _think_ (possibly wrong):
It's based on the V18 patchset


01/19 avcodec,avutil: Move enum AVSubtitleType to avutil, add new and

- major bump in avcodec because AVSubtitleType is removed
- major bump in avutil because AVSubtitleType is added

02/19 avutil/frame: Prepare AVFrame for subtitle handling

- major bump in avutil because AVFrame struct is modified
  (not just adding to the end)

03/19 avcodec/subtitles: Introduce new frame-based subtitle decoding

- minor bump in avcodec. New APIs are added, existing API remains functional

04/19 avfilter/subtitles: Update vf_subtitles to use new decoding api
(no change)

05/19 avcodec,avutil: Move ass helper functions to avutil as avpriv_ 

- major bump in avcodec and avutil
  no public APIs are changed but each lib depends on the other to be updated

06/19 avcodec/subtitles: Migrate subtitle encoders to frame-based API
(no change)
07/19 fftools/play,probe: Adjust for subtitle changes
(no change)
08/19 avfilter/subtitles: Add subtitles.c for subtitle frame allocation
(no change)

09/19 avfilter/avfilter: Handle subtitle frames

- minor bump in avfilter due to additions to the end of AVFilter struct.


No change for the remaining commits (18-19).


Does that make sense?

Thanks,
sw

Quoting Soft Works (2021-12-03 11:25:08)
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > Khirnov
> > Sent: Wednesday, December 1, 2021 2:35 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> > AVSubtitleType
> > 
> > Quoting Soft Works (2021-11-29 20:47:52)
> > > Signed-off-by: softworkz <softworkz@hotmail.com>
> > > ---
> > >  libavcodec/avcodec.h | 19 +--------------
> > >  libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 59 insertions(+), 18 deletions(-)
> > >  create mode 100644 libavutil/subfmt.h
> > 
> > This commit does way more than just moving the enum, it also
> > deprecates existing enum values and adds new ones. That should be
> > mentioned in the commit message and in doc/APIchanges.
> 
> This is something I would need some help with. Which of my commits should 
> bump versions in which libs and where major or minor bump?

If you expect the whole set to be pushed at once, it is enough to have a
separate patch at the end that bumps the versions of all the affected
libraries and adds the necessary APIchanges entries.

> 
> Here's what I _think_ (possibly wrong):
> It's based on the V18 patchset
> 
> 
> 01/19 avcodec,avutil: Move enum AVSubtitleType to avutil, add new and
> 
> - major bump in avcodec because AVSubtitleType is removed
> - major bump in avutil because AVSubtitleType is added

Enums exist only in the headers, they are not exported by the binaries
in the way functions are. So moving an enum declaration from avcodec.h
to lavu does not actually break compatibility as long as avcodec.h
includes the new header (and thus provides the type to any legacy
programs).

So this requires only a minor bump in libavutil for adding a new header
and new enum values. A major bump would be required for changes that
break existing callers, adding new declarations does not break
anyything.

> 
> 02/19 avutil/frame: Prepare AVFrame for subtitle handling
> 
> - major bump in avutil because AVFrame struct is modified
>   (not just adding to the end)

Why not just move your changes to the end?

You seem not to realize that a major bump is a big deal. We typically do
it not more than once per years, and there was one already this spring.
You should avoid API/ABI-breaking changes if possible, and it is very
much possible here.

> 
> 03/19 avcodec/subtitles: Introduce new frame-based subtitle decoding
> 
> - minor bump in avcodec. New APIs are added, existing API remains functional
> 
> 04/19 avfilter/subtitles: Update vf_subtitles to use new decoding api
> (no change)
> 
> 05/19 avcodec,avutil: Move ass helper functions to avutil as avpriv_ 
> 
> - major bump in avcodec and avutil
>   no public APIs are changed but each lib depends on the other to be updated

A minor bump in libavutil. libavcodec must be used with libavutil from
the same commit or newer.

> 
> 06/19 avcodec/subtitles: Migrate subtitle encoders to frame-based API
> (no change)
> 07/19 fftools/play,probe: Adjust for subtitle changes
> (no change)
> 08/19 avfilter/subtitles: Add subtitles.c for subtitle frame allocation
> (no change)
> 
> 09/19 avfilter/avfilter: Handle subtitle frames
> 
> - minor bump in avfilter due to additions to the end of AVFilter struct.

No bump - the changes are in a non-public part of that struct.

Quoting Soft Works (2021-12-03 10:34:29)
> 
> 
> 
> At which place should FF_API_OLD_SUBTITLES get defined then (set to 1 for now)?

I see you figured that out sucessfully in the new version :)

> > > +enum AVSubtitleType {
> > > +
> > > +    /**
> > > +     * Subtitle format unknown.
> > > +     */
> > > +    AV_SUBTITLE_FMT_NONE = -1,
> > > +
> > > +    /**
> > > +     * Subtitle format unknown.
> > > +     */
> > > +    AV_SUBTITLE_FMT_UNKNOWN = 0,
> > > +    SUBTITLE_NONE = 0,          ///< Deprecated, use AV_SUBTITLE_FMT_NONE
> > instead.
> > 
> > This looks suspicious. Are you sure it's not going to break existing
> > code?
> 
> Yes I am. In the legacy API, SUBTITLE_NONE actually had a meaning like 
> "unspecified", "unknown" or "not set" (obviously - having a value of int 0).
> The ***_NONE values of the regular APIs though (e.g. AV_PIXFMT_NONE) 
> have a rather obscure semantic - sometimes being used as array sentinels, 
> sometimes for tristate sef/unset logic etc.

I don't think it's obscure and don't even see the difference --
AV_{PIX,SAMPLE}_FMT_NONE means exactly unspecified/unknown/not set. It's
the equivalent of the null pointer. And exactly like the null pointer,
it can be used as an array terminator.

> 
> SUBTITLE_NONE was nothing like that. It's the logical equivalent to 
> AV_SUBTITLE_FMT_UNKNOWN, despite the naming confusion.

"Nothing like that"? It sounds like exactly the same thing. Also note
that the doxygen for both in your patch says "subtitle format unknown".

> 
> I should also add that the actual define SUBTITLE_NONE has been used at a single
> place in all ffmpeg code only (https://github.com/FFmpeg/FFmpeg/search?q=SUBTITLE_NONE).

And as far as I can tell, it's used for empty subtitle blocks that do
not get output to the user. We do not need a special format for that.

> 
> The naming of the new constants is chosen to follow the API for audio and video
> (AV_SAMPLE_FMT_, AV_PIXFMT_). Actually I tried to establish equality to the
> other two wherever possible, even when something could have been "simplified",
> but I think that uniformity ("know one, know the other") is a much higher value
> from an API perspective than saving two or three functions and a few parameters 
> here and there.

Coherence with the other format APIs is a worthy goal, but seems to me
you could also achieve that by dropping AV_SUBTITLE_FMT_UNKNOWN
completely. What is it actually needed for?

> 
> 
> > > +    /**
> > > +     * Text Formatted as per ASS specification, contained
> > AVSubtitleRect.ass.
> > > +     */
> > > +    AV_SUBTITLE_FMT_ASS = 3,
> > > +    SUBTITLE_ASS = 3,           ///< Deprecated, use AV_SUBTITLE_FMT_ASS
> > instead.
> > > +
> > > +    AV_SUBTITLE_FMT_NB,
> > 
> > The use of this field should be documented. If it's a part of public
> > API, you are preventing any new types from being added without an
> > API/ABI break.
> 
> I guess you mean this text?
> 
>     AV_PIX_FMT_NB         ///< number of pixel formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions

Yes -- if you add new values to the enum then the value of
AV_SUBTITLE_FMT_NB would increase, breaking binary compatibility with
any callers that used it.

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 4:17 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> AVSubtitleType
> 
> Quoting Soft Works (2021-12-03 11:25:08)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > > Khirnov
> > > Sent: Wednesday, December 1, 2021 2:35 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> > > AVSubtitleType
> > >
> > > Quoting Soft Works (2021-11-29 20:47:52)
> > > > Signed-off-by: softworkz <softworkz@hotmail.com>
> > > > ---
> > > >  libavcodec/avcodec.h | 19 +--------------
> > > >  libavutil/subfmt.h   | 58 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 59 insertions(+), 18 deletions(-)
> > > >  create mode 100644 libavutil/subfmt.h
> > >
> > > This commit does way more than just moving the enum, it also
> > > deprecates existing enum values and adds new ones. That should be
> > > mentioned in the commit message and in doc/APIchanges.
> >
> > This is something I would need some help with. Which of my commits should
> > bump versions in which libs and where major or minor bump?
> 
> If you expect the whole set to be pushed at once, it is enough to have a
> separate patch at the end that bumps the versions of all the affected
> libraries and adds the necessary APIchanges entries.

I'm confused, previously I was told that patches should be atomic and I 
thought that APIchanges entries should be made in the same patchset.

Should this be a separate commit then, that only does APIchanges entries
and version bumps?

> > Here's what I _think_ (possibly wrong):
> > It's based on the V18 patchset
> >
> >
> > 01/19 avcodec,avutil: Move enum AVSubtitleType to avutil, add new and
> >
> > - major bump in avcodec because AVSubtitleType is removed
> > - major bump in avutil because AVSubtitleType is added
> 
> Enums exist only in the headers, they are not exported by the binaries
> in the way functions are. So moving an enum declaration from avcodec.h
> to lavu does not actually break compatibility as long as avcodec.h
> includes the new header (and thus provides the type to any legacy
> programs).
> 
> So this requires only a minor bump in libavutil for adding a new header
> and new enum values. A major bump would be required for changes that
> break existing callers, adding new declarations does not break
> anyything.

Sure. Will change that.

> >
> > 02/19 avutil/frame: Prepare AVFrame for subtitle handling
> >
> > - major bump in avutil because AVFrame struct is modified
> >   (not just adding to the end)
> 
> Why not just move your changes to the end?
> 
> You seem not to realize that a major bump is a big deal. We typically do
> it not more than once per years, and there was one already this spring.
> You should avoid API/ABI-breaking changes if possible, and it is very
> much possible here.

The position of the subtitle fields doesn't matter a lot. I thought
that the AVMediaType field would be too elementary to put it at the
end. But on the other hand: If that would be the only change causing 
a major bump, then it will surely make sense to add it to the end
(it could still be moved later when another major bump happens).

> > 03/19 avcodec/subtitles: Introduce new frame-based subtitle decoding
> >
> > - minor bump in avcodec. New APIs are added, existing API remains
> functional
> >
> > 04/19 avfilter/subtitles: Update vf_subtitles to use new decoding api
> > (no change)
> >
> > 05/19 avcodec,avutil: Move ass helper functions to avutil as avpriv_
> >
> > - major bump in avcodec and avutil
> >   no public APIs are changed but each lib depends on the other to be
> updated
> 
> A minor bump in libavutil. libavcodec must be used with libavutil from
> the same commit or newer.

OK, will change.

> > 06/19 avcodec/subtitles: Migrate subtitle encoders to frame-based API
> > (no change)
> > 07/19 fftools/play,probe: Adjust for subtitle changes
> > (no change)
> > 08/19 avfilter/subtitles: Add subtitles.c for subtitle frame allocation
> > (no change)
> >
> > 09/19 avfilter/avfilter: Handle subtitle frames
> >
> > - minor bump in avfilter due to additions to the end of AVFilter struct.
> 
> No bump - the changes are in a non-public part of that struct.

Understood.

So I make the changes, combine them into a single additional commit at the
end, right?

Thanks,
softworkz