[FFmpeg-devel] [PATCH 1/2] avcodec, avformat: deprecate anything related to side data merging

[wm4]

On Sat, 18 Mar 2017 00:15:53 -0300
James Almer <jamrial at gmail.com> wrote:

> On 3/17/2017 11:56 PM, wm4 wrote:
> > On Fri, 17 Mar 2017 23:47:26 -0300
> > James Almer <jamrial at gmail.com> wrote:
> >   
> >> On 3/17/2017 11:38 PM, wm4 wrote:  
> >>> On Fri, 17 Mar 2017 13:59:40 +0100
> >>> Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> >>>     
> >>>> On Fri, Mar 17, 2017 at 12:32 AM, Michael Niedermayer
> >>>> <michael at niedermayer.cc> wrote:    
> >>>>> On Thu, Mar 16, 2017 at 09:20:55PM +0100, Nicolas George wrote:      
> >>>>>> Le sextidi 26 ventôse, an CCXXV, Michael Niedermayer a écrit :      
> >>>>>>> Applications which depend on the current default would need      
> >>>>>>
> >>>>>> ... to implement a correct structure to carry the data from their
> >>>>>> demuxer to the lavc decoders.      
> >>>>>
> >>>>> Is this possible for every application using every framework ?
> >>>>>      
> >>>>
> >>>> If you are using a multimedia framework, then putting non-media data
> >>>> into the media buffers is even worse then doing it internally in
> >>>> avformat->avcodec only.
> >>>> There could be all sorts of other components in play in such a
> >>>> framework which have no idea what this sidedata is, and as someone
> >>>> actually working with such a framework daily (DirectShow), it can and
> >>>> has broken playback when non-avcodec decoders were used and such data
> >>>> was in the packets.    
> >>>
> >>> This was how I actually found out about this. I was feeding
> >>> libavformat data to other non-libavcodec decoders, and there were
> >>> mysterious failures about corrupt data in the packets. I wasn't very
> >>> amused when finding out libavformat deliberately put crap there.
> >>>
> >>> Another time it was because libavformat apparently failed to add side
> >>> data to an AVPacket that should have been there. Again, libavformat's
> >>> "helpful" hack caused trouble.
> >>>
> >>> Not to mention that we sometimes go length to remove minor
> >>> inefficiencies like avoiding copying packets (such as requiring padding
> >>> for bitstream readers), but then copy and reallocate the packet _twice_
> >>> because of the side data merging thing.
> >>>     
> >>>> So in short, the "merge" solution has never solved anything for me
> >>>> using a media framework, luckily it was only used very sparingly
> >>>> (until recently when every packet out of the mpegts demuxer suddenly
> >>>> had pointless sidedata and broke all the things).
> >>>> If you use a multimedia framework that does in no way allow you to
> >>>> attach side data to the packets, then the person working with the
> >>>> framework should decide what to do about that, because only they know
> >>>> how their framework may work - merging it into the data has potential
> >>>> for way more breakage.    
> >>>
> >>> That gives us another nice argument: users of different media
> >>> frameworks might generally not know about this hack, because side data
> >>> is rare. But when a demuxer suddenly adds side data, their code would
> >>> suddenly break for no apparent reason. It would probably take them
> >>> quite some debugging to find out what happens here.
> >>>
> >>> This whole side data merging thing is so god damn ridiculous, and even
> >>> more so because it's actually defended.    
> >>
> >> Lets start by applying the ABI changes so merging doesn't happen
> >> automatically anymore.
> >> The discussion of deprecating and removing the functionality
> >> itself or not shouldn't affect the above.  
> > 
> > I hope so - that's why I separated them in the first place.  
> 
> I mean separate patches. The ABI changes can go in right now or as
> soon as we bump major version. That alone is enough to get most of
> the headaches out of the way.
> 
> > 
> > Deprecation of functions doesn't mean much anyway, since we apparently
> > wait a whole 2 years (?) until removing them.  
> 
> If we deprecate the functions it's something that will be reported
> and reflected in APIChanges. If we then decide to change that then
> the deprecation reversal also needs to be reported in APIChanges.
> So to avoid the noise, better to make sure we all agree on the
> deprecation from the beginning.

I'd rather push the patch as-is. Whether someone wants to un-deprecate
those 2 functions is a different question.

From what I can see, nobody has actually rejected these patches, not
even Michael Niedermayer.