[FFmpeg-devel] [PATCH 2/2] Move AVSubtitle from lavc to lavu

[Clément Bœsch]

On Mon, Dec 15, 2014 at 10:49:47AM +0100, wm4 wrote:
> On Mon, 15 Dec 2014 10:38:01 +0100
> Clément Bœsch <u at pkh.me> wrote:
> 
> > On Mon, Dec 15, 2014 at 10:23:33AM +0100, wm4 wrote:
> > > On Sun, 14 Dec 2014 16:47:44 +0100
> > > Clément Bœsch <u at pkh.me> wrote:
> > > 
> > > > ---
> > > >  doc/APIchanges       |   5 +++
> > > >  libavcodec/avcodec.h |  63 +++---------------------------
> > > >  libavcodec/utils.c   |  29 +++-----------
> > > >  libavcodec/version.h |   3 ++
> > > >  libavutil/Makefile   |   2 +
> > > >  libavutil/subtitle.c |  62 +++++++++++++++++++++++++++++
> > > >  libavutil/subtitle.h | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  7 files changed, 191 insertions(+), 80 deletions(-)
> > > >  create mode 100644 libavutil/subtitle.c
> > > >  create mode 100644 libavutil/subtitle.h
> > > > 
> > > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > > index ba7eae1..989794a 100644
> > > > --- a/doc/APIchanges
> > > > +++ b/doc/APIchanges
> > > > @@ -15,6 +15,11 @@ libavutil:     2014-08-09
> > > >  
> > > >  API changes, most recent first:
> > > >  
> > > > +2014-12-xx - xxxxxxx - lavc 56.16.100 / lavu 54.17.100 - lavc/avcodec.h lavu/picture.h
> > > > +  Move AVSubtitle definition from libavcodec to libavutil and add
> > > > +  av_subtitle_*() functions which should be used instead of stack allocation
> > > > +  and the now deprecated avsubtitle_free().
> > > > +
> > > 
> > > "should"?
> > > 
> > > Can this requirement be lifted/delayed until there's actually a reason
> > > to do so?
> > 
> > The structure is unchanged for now, so yeah it's "should" (as a prevention
> > for the future). I don't plan to add a field that soon, but it's highly
> > recommended. Maybe I can change the "should" into "must" and saying that
> > for now it has no impact, but will in the future.
> > 
> > > At that point, the struct should probably be renamed, to
> > > avoid turning valid code into subtly broken code merely by adding API
> > > compatible implementation details.
> > 
> > There are actually 2 changes in this code, and maybe I should split. But
> > basically the addition of the functions is kind of unrelated to the move.
> > It's just a security for us for later, when we will be willing to add a
> > field (if we ever do).
> > 
> > We should have created these functions a long time ago, but the fact that
> > AVSubtitle actually belong into libavutil prevented us from doing it:
> > basically, if we had added these helpers, the move to libavutil would have
> > been way more complicated (moving a structure definition is way easier
> > than moving a structure definition AND its functions).
> > 
> > [...]
> > 
> 
> My point is: barely anyone (applications) will notice this API change.
> It will go unnoticed, or there will be no perceived need to update the
> code. And then when you actually extend this stuff, you will start
> breaking application code that worked before.

What do you suggest to make people notice the API change? Currently
AVSubtitle has one single function, which I deprecated.

So you think we should introduce a AVSubtitle2 into libavutil instead? I
don't like that that much TBH, especially since it will require me to name
the new functions av_subtitle2_* to make it consistent.

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141215/29a0a449/attachment.asc>