[FFmpeg-devel] FFmpeg 3.3

wm4 nfxjfg at googlemail.com
Tue Mar 14 19:09:57 EET 2017


On Tue, 14 Mar 2017 13:50:53 -0300
James Almer <jamrial at gmail.com> wrote:

> On 3/14/2017 1:33 PM, 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?
> > 
> > Can't you see what a giant heap of bullshit this is?  
> 
> I can see your point for other fields in other structs but in here
> size_t is simply wrong. It should have never been committed like
> this to begin with.

Why is it "simply wrong"?

> We could try to get libav to change it as well, but if they don't
> then at the very least this one (very obscure) struct will have to
> diverge between projects.

What about AVFrame.crop_top etc.?


More information about the ffmpeg-devel mailing list