[FFmpeg-devel] [PATCH v4 05/18] avcodec/avcodec: Remove sub_text_format parameter

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Saturday, 11 September 2021 12:39
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v4 05/18] avcodec/avcodec: Remove
> sub_text_format parameter
> 
> Soft Works:
> > It has never been used and going forward, there's no perspective
> > that it would ever be.
> >
> > Signed-off-by: softworkz <softworkz at hotmail.com>
> > ---
> >  libavcodec/avcodec.h       | 8 --------
> >  libavcodec/options_table.h | 2 --
> >  libavcodec/version.h       | 4 ++--
> >  3 files changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 0f71b42d61..01c881e6d1 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -1854,14 +1854,6 @@ typedef struct AVCodecContext {
> >       */
> >      AVBufferRef *hw_frames_ctx;
> >
> > -    /**
> > -     * Control the form of AVSubtitle.rects[N]->ass
> > -     * - decoding: set by user
> > -     * - encoding: unused
> > -     */
> > -    int sub_text_format;
> > -#define FF_SUB_TEXT_FMT_ASS              0
> > -
> >      /**
> >       * Audio only. The amount of padding (in samples) appended by
> the encoder to
> >       * the end of the audio. I.e. this number of decoded samples
> must be
> > diff --git a/libavcodec/options_table.h
> b/libavcodec/options_table.h
> > index ae42b65b7b..8a61154653 100644
> > --- a/libavcodec/options_table.h
> > +++ b/libavcodec/options_table.h
> > @@ -366,8 +366,6 @@ static const AVOption avcodec_options[] = {
> >  {"auto",        NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
> FF_SUB_CHARENC_MODE_AUTOMATIC},   INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"},
> >  {"pre_decoder", NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
> FF_SUB_CHARENC_MODE_PRE_DECODER}, INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"},
> >  {"ignore",      NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
> FF_SUB_CHARENC_MODE_IGNORE},      INT_MIN, INT_MAX, S|D,
> "sub_charenc_mode"},
> > -{"sub_text_format", "set decoded text subtitle format",
> OFFSET(sub_text_format), AV_OPT_TYPE_INT, {.i64 =
> FF_SUB_TEXT_FMT_ASS}, 0, 1, S|D, "sub_text_format"},
> > -{"ass",              NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
> FF_SUB_TEXT_FMT_ASS},              INT_MIN, INT_MAX, S|D,
> "sub_text_format"},
> >  {"apply_cropping", NULL, OFFSET(apply_cropping), AV_OPT_TYPE_BOOL,
> { .i64 = 1 }, 0, 1, V | D },
> >  {"skip_alpha", "Skip processing alpha", OFFSET(skip_alpha),
> AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, V|D },
> >  {"field_order", "Field order", OFFSET(field_order),
> AV_OPT_TYPE_INT, {.i64 = AV_FIELD_UNKNOWN }, 0, 5, V|D|E,
> "field_order" },
> > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > index 4b4fe543ab..0fa5b1a72a 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -28,8 +28,8 @@
> >  #include "libavutil/version.h"
> >
> >  #define LIBAVCODEC_VERSION_MAJOR  59
> > -#define LIBAVCODEC_VERSION_MINOR   7
> > -#define LIBAVCODEC_VERSION_MICRO 103
> > +#define LIBAVCODEC_VERSION_MINOR   8
> > +#define LIBAVCODEC_VERSION_MICRO 100
> >
> >  #define LIBAVCODEC_VERSION_INT
> AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> >
> LIBAVCODEC_VERSION_MINOR, \
> >
> 
> It is wrong that this has not been used: It was used during the
> switch
> away from ass with inline timing (as git pickaxe would have told
> you).
> But just because it is unused now does not mean that you can just
> remove
> it as it is part of the public API. It needs a deprecation. Would you
> mind if I do this?

Yes please, that would be great! 

> (ffmpeg.c even still sets this option which you
> only
> remove in 8/18 (which means fate will be totally red for this and the
> next patch as ffmpeg errors out on unrecognized options).)

I had thought that FATE needs pass only for the whole patchset
not for individual commits.

Shouldn't commits be separated between libraries (and fftools)?
Would it be allowed to do the removal (from 8/18) in this patch (5/18)?
Or should I add an additional preceding commit for the removal?

Thank you very much,
softworkz