[FFmpeg-devel] [PATCH] all: avoid data imports across DLL boundaries

[wm4]

On Thu, 24 Aug 2017 11:20:17 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Thu, Aug 24, 2017 at 10:52:55AM +0200, wm4 wrote:
> > On Wed, 23 Aug 2017 19:23:12 -0300
> > James Almer <jamrial at gmail.com> wrote:
> >   
> > > On 8/21/2017 2:51 PM, wm4 wrote:  
> > > > From: Pedro Pombeiro <pedropombeiro at gmail.com>
> > > > 
> > > > DLL data imports cause problems on Windows. They normally require
> > > > each variable to be correctly marked with dllimport/dllexport
> > > > declspecs. For MSVC, we define av_export to dllimport declspec. This
> > > > is not entirely correct - depending on which sub-lib is built, they
> > > > should be marked to dllexport instead. It happens to work with MSVC,
> > > > because it supports exports incorrectly marked as imports. Trying to
> > > > use this breaks on MinGW and results in linker errors.
> > > > 
> > > > On MinGW, not using any import/export specifiers happens to work,
> > > > because binutils and the MinGW runtime provide "pseudo relocations",
> > > > which manually fix these imports at load time. This does not work with
> > > > MinGW WinRT builds: the relocations cannot be performed because this
> > > > would require writing to the code section, which is not allowed.
> > > > 
> > > > Get rid of all these issues by not using data exports/imports. The
> > > > public API is already free of them, but avpriv_ symbols make extensive
> > > > use of them. Replace them all with getters.    
> > > 
> > > Should be good i think, but it can't be applied as is until the next
> > > major bump as it breaks ABI (Tons of avpriv_ symbols are removed).  
> > 
> > Well, I call bullshit. We've never taken ABI compatibility between
> > FFmpeg libs seriously, especially not if it was about avpriv functions.  
> 
> We did take ABI compatibility between FFmpeg libs serious.
> And we must to be shiped by distributions that package the libs based
> on our ABI versions.
> 
> 
> >
> > And guess what? You didn't either. Commit 7c9d2ad45f4e46ad2c3b2 is
> > authored and committed by you, and changes an avpriv function in an
> > incompatible way, without even minor version bumps.  
> 
> This commit did not break ABI
> no release contains the removed symbol:
> 
>  git grep avpriv_dca_parse_core_frame_header release/3.3
>  git grep avpriv_dca_parse_core_frame_header release/3.2
>  git grep avpriv_dca_parse_core_frame_header release/3.1
>  git grep avpriv_dca_parse_core_frame_header release/3.0
>  git grep avpriv_dca_parse_core_frame_header release/2.4
>  git grep avpriv_dca_parse_core_frame_header release/2.8
> 
> The symbol was added 9 days before it was removed, it was in no
> release
> 
> commit 7c9d2ad45f4e46ad2c3b2e93051efbe1e0d0529e
> Author: James Almer <jamrial at gmail.com>
> Date:   Wed Jul 19 01:53:22 2017 -0300
> 
>     avcodec/dca: remove GetBitContext usage from avpriv_dca_parse_core_frame_header()
> 
> commit 2123ddb4251bf39bde8b38a1307a0f6154d260e6
> Author: foo86 <foobaz86 at gmail.com>
> Date:   Mon Jul 10 17:11:33 2017 +0300
> 
>     avcodec: add avpriv_dca_parse_core_frame_header()
> [...]

We keep ABI stability even within git master, not just within releases.
Otherwise, our lives would be so much easier whenever ABI problems come
up.