[FFmpeg-devel] [PATCH v15 01/16] global: Prepare AVFrame for subtitle handling

[Anton Khirnov]

Quoting Soft Works (2021-11-25 18:20:31)
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Anton
> > Khirnov
> > Sent: Thursday, November 25, 2021 4:56 PM
> > To: ffmpeg-devel at ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v15 01/16] global: Prepare AVFrame for
> > subtitle handling
> > 
> > Quoting Soft Works (2021-11-25 01:48:11)
> > > Root commit for adding subtitle filtering capabilities.
> > > In detail:
> > >
> > > - Add type (AVMediaType) field to AVFrame
> > >   Replaces previous way of distinction which was based on checking
> > >   width and height to determine whether a frame is audio or video
> > > - Add subtitle fields to AVFrame
> > > - Add new struct AVSubtitleArea, similar to AVSubtitleRect, but different
> > >   allocation logic. Cannot and must not be used interchangeably, hence
> > >   the new struct
> > > - Move enum AVSubtitleType, AVSubtitle and AVSubtitleRect to avutil
> > > - Add public-named members to enum AVSubtitleType (AV_SUBTITLE_FMT_)
> > > - Add avcodec_decode_subtitle3 which takes subtitle frames,
> > >   serving as compatibility shim to legacy subtitle decoding
> > > - Add additional methods for conversion between old and new API
> > >
> > > Signed-off-by: softworkz <softworkz at hotmail.com>
> > > ---
> > >  libavcodec/avcodec.c       |  19 ---
> > >  libavcodec/avcodec.h       |  75 ++----------
> > >  libavcodec/decode.c        |  53 ++++++--
> > >  libavcodec/pgssubdec.c     |   1 +
> > >  libavcodec/utils.c         |  11 ++
> > >  libavfilter/vf_subtitles.c |  50 ++++++--
> > >  libavformat/utils.c        |   1 +
> > >  libavutil/Makefile         |   2 +
> > >  libavutil/frame.c          | 194 ++++++++++++++++++++++++++---
> > >  libavutil/frame.h          |  93 +++++++++++++-
> > >  libavutil/subfmt.c         | 243 +++++++++++++++++++++++++++++++++++++
> > >  libavutil/subfmt.h         | 185 ++++++++++++++++++++++++++++
> > >  12 files changed, 802 insertions(+), 125 deletions(-)
> > >  create mode 100644 libavutil/subfmt.c
> > >  create mode 100644 libavutil/subfmt.h
> > 
> > First of all, this should be split. It does way way more than just
> > "prepare AVFrame". It adds random things all over the place, some of
> > them completely spurious (like the single include in pgssubdec).
> 
> 
> I double-checked. It cannot be split without breaking compilation.

This cannot be true. And if you really want to claim that it is, you
better present some arguments.

At the very least, adding new functions and AVFrame fields cannot affect
any other code and can be done in its own patch. I'm pretty sure other
things can be split as well.

-- 
Anton Khirnov