[FFmpeg-devel] [PATCH] lavfi/drawtext: replace "draw" option with generic timeline interface.

[Alexander Strasser]

On 2013-08-23 22:21 +0200, Clément Bœsch wrote:
> On Fri, Aug 23, 2013 at 09:29:09PM +0200, Alexander Strasser wrote:
> > Hi Clément!
> > 
> > On 2013-08-21 01:26 +0200, Clément Bœsch wrote:
> > > On Tue, Aug 20, 2013 at 11:00:47PM +0000, Paul B Mahol wrote:
> > > > On 8/20/13, Clement Boesch <u at pkh.me> wrote:
> > > > > ---
> > > > >  Changelog                 |  1 +
> > > > >  doc/filters.texi          | 12 +-----------
> > > > >  libavfilter/version.h     |  2 +-
> > > > >  libavfilter/vf_drawtext.c | 17 +++--------------
> > > > >  4 files changed, 6 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/Changelog b/Changelog
> > > > > index 4a6c60c..4337ba2 100644
> > > > > --- a/Changelog
> > > > > +++ b/Changelog
> > > > > @@ -15,6 +15,7 @@ version <next>
> > > > >    data read from an input file
> > > > >  - incomplete Voxware MetaSound decoder
> > > > >  - read EXIF metadata from JPEG
> > > > > +- drawtext filter now uses the generic timeline interface
> > > > >
> > > > >
> > > > 
> > > > What about backward compatibility?
> > > 
> > > I guess I'm being a bit rude assuming micro bump and Changelog entry are
> > > enough. Should I add some version #ifdefery around all the "draw" code
> > > which will be removed at next major bump? It sounded a bit overkill to me
> > > but well...
> > 
> >   Counter proposal attached; please start flaming :)
> > 
> 
> That diff was basically my original patch, but I didn't like keeping both
> options.
> 
> >   BTW there is even another possibility. Make "enable" and
> > "draw" work together without prioritizing "enable" but by
> > treating both equally and combine them with a logical or.
> > 
> 
> If I understand you well, that is a bit tricky to do.

  Not really if I am not mistaken. I had it working here
by using TIMELINE_INTERNAL, but considered the behaviour
less expected. So I switched over to TIMELINE_GENERIC again
because it minimizes the changes a little.
  
> >   I do mind dropping draw option because of two things:
> > 
> >   - compatibility
> 
> Well I can come up with a patch for that in a few minutes, by adding
> #ifdefery in the filter + av_log warning about "draw" when used. Assuming
> this, next lavfi major bump will kill the option. It will just clutter a
> bit the filter code for a while.
> 
> Maybe we should consider a new way of deprecating
> API/ABI/whatever-interfaces so we don't have to bother about such things
> in the future; it's kind of a drag to deal with such problem regularly.

  I agree that deprecating APIs on a kind of regular basis is
tiresome. But I believe there is more to it (sometimes). Probably
we need a process change regarding the changes of public APIs or
for that matter any public interfaces.

> >   - functionality
> >     (draw option can use information timeline can't e.g. text width/height)
> > 
> 
> Does that matter in practice?

  Depends on your use-case I would say. If I would be using it today
and it breaks my application tomorrow or in a few weeks I would quite
certainly be annoyed!

  My point is: When you are a user of FFmpeg or lavfi you have lots
of other stuff to care about; not FFmpeg related at all. So every
disruption will make you increase a mental counter (those are famous
for not being really accurate/correct) for that component. So naturally
if FFmpeg has the highscore you will not be positive about it. This is
not made up, changing APIs and lacking docs are probably the number one
complaints expressed to me in person. Though that is another mental
counter and of course not representative in any way. I am just saying
this is not a purely theoretical debate.

  Anyway back the actual topic--- I do not see why this functionality
has to be removed. It is neither huge nor complex; might even be useful
to extend it in future. I might very well be overlooking something. Am
I missing something essential?

  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130823/cde5e7c1/attachment.asc>