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.
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
