[FFmpeg-devel] [PATCH v15 01/16] global: Prepare AVFrame for subtitle handling
Soft Works
softworkz at hotmail.com
Thu Nov 25 19:20:31 EET 2021
> -----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.
What this commit does is to relocate and promote AVSubtitle and
AVSubtitleRect to the public API - alongside the new AVSubtitleArea
(the replacement for AVSubtitleRect. AVFrame is the replacement for
AVSubtitle)
AVSubtitle and AVSubtitleRect are immediately marked deprecated,
but that's the only way to make the transition and retain compatibility.
Same promotion happens for the subtitle enum which transitions from
e.g. SUBTITLE_BITMAP to AV_SUBTITLE_FMT_BITMAP. Previous constants
are kept for compatibility.
For this promotion, the code files need to be relocated to libavutil.
This in turn requires some header changes.
All files in this commit are depending on those references and
need to be changed in a single commit.
The include change to pgssubdec is not spurious but required.
Same is needed for the teletext decoder (which I had forgotten).
Why exactly those two codecs? => because these are the two that
can handle both, text and bitmap subs, that's why they need the
subtitle type enum constants.
Andreas hat reviewed this part a while ago and this is the
result we ended up with after a few iterations.
Kind regards,
softworkz
>
> >
> > +/**
> > + * Return subtitle format from a codec descriptor
> > + *
> > + * @param codec_descriptor codec descriptor
> > + * @return the subtitle type (e.g. bitmap, text)
> > + */
> > +enum AVSubtitleType av_get_subtitle_format_from_codecdesc(const
> AVCodecDescriptor *codec_descriptor);
>
> New functions should be namespaced along the lines of av(_)<object>_<action>
> In this case, something like avcodec_descriptor_get_subtitle_format()
>
> Also, it seems to introduce an assumption that bitmap and text are
> mutually exclusive, while descriptors treat them as flags.
>
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 753234792e..742f4ba07e 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -34,6 +34,7 @@
> > #include "rational.h"
> > #include "samplefmt.h"
> > #include "pixfmt.h"
> > +#include "subfmt.h"
> > #include "version.h"
> >
> >
> > @@ -278,7 +279,7 @@ typedef struct AVRegionOfInterest {
> > } AVRegionOfInterest;
> >
> > /**
> > - * This structure describes decoded (raw) audio or video data.
> > + * This structure describes decoded (raw) audio, video or subtitle data.
> > *
> > * AVFrame must be allocated using av_frame_alloc(). Note that this only
> > * allocates the AVFrame itself, the buffers for the data must be managed
> > @@ -309,6 +310,13 @@ typedef struct AVRegionOfInterest {
> > */
> > typedef struct AVFrame {
> > #define AV_NUM_DATA_POINTERS 8
> > + /**
> > + * Media type of the frame (audio, video, subtitles..)
> > + *
> > + * See AVMEDIA_TYPE_xxx
> > + */
> > + enum AVMediaType type;
> > +
> > /**
> > * pointer to the picture/channel planes.
> > * This might be different from the first allocated byte. For video,
> > @@ -390,7 +398,7 @@ typedef struct AVFrame {
> > /**
> > * format of the frame, -1 if unknown or unset
> > * Values correspond to enum AVPixelFormat for video frames,
> > - * enum AVSampleFormat for audio)
> > + * enum AVSampleFormat for audio, AVSubtitleType for subtitles)
> > */
> > int format;
> >
> > @@ -481,6 +489,39 @@ typedef struct AVFrame {
> > */
> > uint64_t channel_layout;
> >
> > + /**
> > + * Display start time, relative to packet pts, in ms.
> > + */
> > + uint32_t subtitle_start_time;
> > +
> > + /**
> > + * Display end time, relative to packet pts, in ms.
> > + */
> > + uint32_t subtitle_end_time;
> > +
> > + /**
> > + * Number of items in the @ref subtitle_areas array.
> > + */
> > + unsigned num_subtitle_areas;
> > +
> > + /**
> > + * Array of subtitle areas, may be empty.
> > + */
> > + AVSubtitleArea **subtitle_areas;
> > +
> > + /**
> > + * Usually the same as packet pts, in AV_TIME_BASE.
> > + *
> > + * @deprecated This is kept for compatibility reasons and corresponds
> to
> > + * AVSubtitle->pts. Might be removed in the future.
> > + */
> > + int64_t subtitle_pts;
> > +
> > + /**
> > + * Header containing style information for text subtitles.
> > + */
> > + AVBufferRef *subtitle_header;
>
> This is breaking ABI.
>
> --
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list