[FFmpeg-devel] FFmpeg 3.3

[Clément Bœsch]

On Tue, Mar 14, 2017 at 05:33:41PM +0100, wm4 wrote:
> On Tue, 14 Mar 2017 17:02:34 +0100
> Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> 
> > On Tue, Mar 14, 2017 at 4:49 PM, wm4 <nfxjfg at googlemail.com> wrote:
> > > On Tue, 14 Mar 2017 14:19:24 +0000
> > > Rostislav Pehlivanov <atomnuker at gmail.com> wrote:
> > >  
> > >> On 14 March 2017 at 13:38, wm4 <nfxjfg at googlemail.com> wrote:
> > >>  
> > >> > On Tue, 14 Mar 2017 10:24:10 -0300
> > >> > James Almer <jamrial at gmail.com> wrote:
> > >> >  
> > >> > > On 3/14/2017 9:31 AM, Rostislav Pehlivanov wrote:  
> > >> > > > On 14 March 2017 at 11:29, Michael Niedermayer <michael at niedermayer.cc  
> > >> > >  
> > >> > > > wrote:
> > >> > > >  
> > >> > > >>
> > >> > > >> What about the spherical API size_t / uint32_t issue ?
> > >> > > >> we can change it before the release but not after it
> > >> > > >>
> > >> > > >>  
> > >> > > > IMO its better to have the same type on all platforms. It also doesn't  
> > >> > make  
> > >> > > > sense to use size_t for something describing a position.  
> > >> > >
> > >> > > wm4 argued it would generate problems for being different than libav's.
> > >> > > We don't care about ABI compatibility anymore so struct field size or
> > >> > > position isn't a problem.
> > >> > >
> > >> > > What kind of trouble would having the function signatures use uint32_t
> > >> > > here and size_t on libav generate? It's technically breaking API
> > >> > > compatibility i guess.  
> > >> >
> > >> > I'm mostly thinking about things like overflow checks. And of course,
> > >> > you know, the damn user API.
> > >> >
> > >> >  
> > >> Since the only way to currently get the (float) value of the position on
> > >> any platform is to measure its size, convert it to bits and calculate the
> > >> limit and divide it, changing the type to a static wouldn't cause any
> > >> problems for someone already doing this and will keep compatibility with
> > >> libav. Someone who assumes size_t is always going to be 64 or 32 bits will
> > >> be disappointed when their code doesn't work on MIPS/32 bit stuff but its
> > >> their fault for incorrectly assuming that and its our fault for letting
> > >> this patch in with size_t in the first place, so we ought to fix it.  
> > >
> > > Feel free to send a patch to Libav. Hopefully I won't be the one who
> > > allows subtle and pointless API divergence over silly reasons.  
> > 
> > Using "API divergence" as an excuse to accept silly decision is
> > equally silly, if not more so.
> 
> Well, you're not the one who will have to deal with an increasing
> number of different basic types on the same struct fields.
> 
> If we do this now, you can bet we'll change AVFrame.crop* from size_t
> to something else too. And then if Libav changes AVFrame.width/height
> to size_t too (like they apparently plan to), it will be even worse.
> You probably don't care about this, because you won't have to deal with
> the end result, but I'm not only going to have to use this subtle
> different API, but I'll also probably contribute patches to both
> projects, and then I'll have to change my patches to use the "correct"
> type on each side. And what about potential project reunification?
> Another idiotic thing we'll have to flame about?
> 

We are diverging more and more API wise. Feel free to correct me, but I
thought importing or not a change from Libav was dependent on two main
factors:

- how much effort it will cause in a following merge?
- is it technically a good change?

Beside the merge commits, with time we also diverged a lot in API, and not
only in API additions, but also on internal breaking changes (typically in
lavfi but not limited to).

We know it's a problem for users who want to support both, but on the
other hand, we can't always be shaped by Libav decisions, there is a need
for a technical evaluations of the different solutions.

Now back to the current issue.

Do you agree with the fact that uint32_t is a better technical choice?
(I think the main argument is that it's a pain to have proper FATE
coverage if you don't?)

If so, I'm assuming you believe that even if it's better, it's not worth
the breaking change for users. I do not share the feeling at all if it
reduces FATE coverage. If you share that sentiment, maybe it's worth
fighting for that technical argument where you want it to change. Why
would FFmpeg follow Libav even if you disagree with Libav's argument?

If not, then I think it's better to defend your technical disagreement
instead of using a political one. And maybe that size_t BS is going to
have acceptance here in FFmpeg and the problem will be solved that way.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170314/b250da21/attachment.sig>