diff mbox

[FFmpeg-devel] avutil/internal: Document ugly looking windows specific file related function renaming

Message ID 20170201133550.14771-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Feb. 1, 2017, 1:35 p.m. UTC
Found-by: ubitux
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavutil/internal.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

wm4 Feb. 1, 2017, 2:26 p.m. UTC | #1
On Wed,  1 Feb 2017 14:35:50 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> Found-by: ubitux
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/internal.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavutil/internal.h b/libavutil/internal.h
> index a19975d474..e97034887b 100644
> --- a/libavutil/internal.h
> +++ b/libavutil/internal.h
> @@ -243,8 +243,12 @@ void avpriv_request_sample(void *avc,
>  #pragma comment(linker, "/include:" EXTERN_PREFIX "avpriv_snprintf")
>  #endif
>  
> +// Rename shared function (both use and implementation of them) so they are not shared
> +// and each library carries its own copy of a implementation, this is needed as

So why are they not appropriately named in the first place?

> +// the fd numbers are not transportable between libs on windows

AFAIK they are, as long as they're linked to the same stdlib (which
probably is the case for libav*).

>  #define avpriv_open ff_open
>  #define avpriv_tempfile ff_tempfile
> +
>  #define PTRDIFF_SPECIFIER "Id"
>  #define SIZE_SPECIFIER "Iu"
>  #else
Michael Niedermayer Feb. 1, 2017, 9:24 p.m. UTC | #2
On Wed, Feb 01, 2017 at 03:26:45PM +0100, wm4 wrote:
> On Wed,  1 Feb 2017 14:35:50 +0100
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> > Found-by: ubitux
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavutil/internal.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/libavutil/internal.h b/libavutil/internal.h
> > index a19975d474..e97034887b 100644
> > --- a/libavutil/internal.h
> > +++ b/libavutil/internal.h
> > @@ -243,8 +243,12 @@ void avpriv_request_sample(void *avc,
> >  #pragma comment(linker, "/include:" EXTERN_PREFIX "avpriv_snprintf")
> >  #endif
> >  
> > +// Rename shared function (both use and implementation of them) so they are not shared
> > +// and each library carries its own copy of a implementation, this is needed as
> 
> So why are they not appropriately named in the first place?
> 
> > +// the fd numbers are not transportable between libs on windows
> 
> AFAIK they are, as long as they're linked to the same stdlib (which
> probably is the case for libav*).

here is the commit introducing the first function using this system.
I belive its commit message is quite good and explains the reasoning.
Beyond that iam the wrong one to discuss this with, iam not a windows
developer and i didnt design this, which is not to say i like or
dislike it. I just added one function IIRC and i wanted to document it
as ubitux seemed to be confused for a moment when seeing it the first
time

commit e743e7ae6ee7e535c4394bec6fe6650d2b0dbf65
Author: Martin Storsjö <martin@martin.st>
Date:   Fri Aug 9 11:06:46 2013 +0300

    libavutil: Make avpriv_open a library-internal function on msvcrt

    Add one copy of the function into each of the libraries, similarly
    to what we do for log2_tab. When using static libs, only one
    copy of the file_open.o object file gets included, while when
    using shared libraries, each of them get a copy of its own.

    This fixes DLL builds with a statically linked C runtime, where
    each DLL effectively has got its own instance of the C runtime,
    where file descriptors can't be shared across runtimes.

    On systems not using msvcrt, the function is not duplicated.

[...]
wm4 Feb. 2, 2017, 8:07 a.m. UTC | #3
On Wed, 1 Feb 2017 22:24:14 +0100
Michael Niedermayer <michaelni@gmx.at> wrote:

> On Wed, Feb 01, 2017 at 03:26:45PM +0100, wm4 wrote:
> > On Wed,  1 Feb 2017 14:35:50 +0100
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> > > Found-by: ubitux
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavutil/internal.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/libavutil/internal.h b/libavutil/internal.h
> > > index a19975d474..e97034887b 100644
> > > --- a/libavutil/internal.h
> > > +++ b/libavutil/internal.h
> > > @@ -243,8 +243,12 @@ void avpriv_request_sample(void *avc,
> > >  #pragma comment(linker, "/include:" EXTERN_PREFIX "avpriv_snprintf")
> > >  #endif
> > >  
> > > +// Rename shared function (both use and implementation of them) so they are not shared
> > > +// and each library carries its own copy of a implementation, this is needed as  
> > 
> > So why are they not appropriately named in the first place?
> >   
> > > +// the fd numbers are not transportable between libs on windows  
> > 
> > AFAIK they are, as long as they're linked to the same stdlib (which
> > probably is the case for libav*).  
> 
> here is the commit introducing the first function using this system.
> I belive its commit message is quite good and explains the reasoning.
> Beyond that iam the wrong one to discuss this with, iam not a windows
> developer and i didnt design this, which is not to say i like or
> dislike it. I just added one function IIRC and i wanted to document it
> as ubitux seemed to be confused for a moment when seeing it the first
> time
> 
> commit e743e7ae6ee7e535c4394bec6fe6650d2b0dbf65
> Author: Martin Storsjö <martin@martin.st>
> Date:   Fri Aug 9 11:06:46 2013 +0300
> 
>     libavutil: Make avpriv_open a library-internal function on msvcrt
> 
>     Add one copy of the function into each of the libraries, similarly
>     to what we do for log2_tab. When using static libs, only one
>     copy of the file_open.o object file gets included, while when
>     using shared libraries, each of them get a copy of its own.
> 
>     This fixes DLL builds with a statically linked C runtime, where
>     each DLL effectively has got its own instance of the C runtime,
>     where file descriptors can't be shared across runtimes.
> 
>     On systems not using msvcrt, the function is not duplicated.
> 
> [...]

Maybe this would be better:

// If each libav* DLL is statically linked to the C runtime, FDs can
// not be used across library boundaries. Duplicate these functions in
// each DLL to avoid this problem.
Michael Niedermayer Feb. 3, 2017, 11:06 a.m. UTC | #4
On Thu, Feb 02, 2017 at 09:07:25AM +0100, wm4 wrote:
> On Wed, 1 Feb 2017 22:24:14 +0100
> Michael Niedermayer <michaelni@gmx.at> wrote:
> 
> > On Wed, Feb 01, 2017 at 03:26:45PM +0100, wm4 wrote:
> > > On Wed,  1 Feb 2017 14:35:50 +0100
> > > Michael Niedermayer <michael@niedermayer.cc> wrote:
> > >   
> > > > Found-by: ubitux
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >  libavutil/internal.h | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/libavutil/internal.h b/libavutil/internal.h
> > > > index a19975d474..e97034887b 100644
> > > > --- a/libavutil/internal.h
> > > > +++ b/libavutil/internal.h
> > > > @@ -243,8 +243,12 @@ void avpriv_request_sample(void *avc,
> > > >  #pragma comment(linker, "/include:" EXTERN_PREFIX "avpriv_snprintf")
> > > >  #endif
> > > >  
> > > > +// Rename shared function (both use and implementation of them) so they are not shared
> > > > +// and each library carries its own copy of a implementation, this is needed as  
> > > 
> > > So why are they not appropriately named in the first place?
> > >   
> > > > +// the fd numbers are not transportable between libs on windows  
> > > 
> > > AFAIK they are, as long as they're linked to the same stdlib (which
> > > probably is the case for libav*).  
> > 
> > here is the commit introducing the first function using this system.
> > I belive its commit message is quite good and explains the reasoning.
> > Beyond that iam the wrong one to discuss this with, iam not a windows
> > developer and i didnt design this, which is not to say i like or
> > dislike it. I just added one function IIRC and i wanted to document it
> > as ubitux seemed to be confused for a moment when seeing it the first
> > time
> > 
> > commit e743e7ae6ee7e535c4394bec6fe6650d2b0dbf65
> > Author: Martin Storsjö <martin@martin.st>
> > Date:   Fri Aug 9 11:06:46 2013 +0300
> > 
> >     libavutil: Make avpriv_open a library-internal function on msvcrt
> > 
> >     Add one copy of the function into each of the libraries, similarly
> >     to what we do for log2_tab. When using static libs, only one
> >     copy of the file_open.o object file gets included, while when
> >     using shared libraries, each of them get a copy of its own.
> > 
> >     This fixes DLL builds with a statically linked C runtime, where
> >     each DLL effectively has got its own instance of the C runtime,
> >     where file descriptors can't be shared across runtimes.
> > 
> >     On systems not using msvcrt, the function is not duplicated.
> > 
> > [...]
> 

> Maybe this would be better:
> 
> // If each libav* DLL is statically linked to the C runtime, FDs can
> // not be used across library boundaries. Duplicate these functions in
> // each DLL to avoid this problem.

LGTM

thx

[...]
diff mbox

Patch

diff --git a/libavutil/internal.h b/libavutil/internal.h
index a19975d474..e97034887b 100644
--- a/libavutil/internal.h
+++ b/libavutil/internal.h
@@ -243,8 +243,12 @@  void avpriv_request_sample(void *avc,
 #pragma comment(linker, "/include:" EXTERN_PREFIX "avpriv_snprintf")
 #endif
 
+// Rename shared function (both use and implementation of them) so they are not shared
+// and each library carries its own copy of a implementation, this is needed as
+// the fd numbers are not transportable between libs on windows
 #define avpriv_open ff_open
 #define avpriv_tempfile ff_tempfile
+
 #define PTRDIFF_SPECIFIER "Id"
 #define SIZE_SPECIFIER "Iu"
 #else