Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-09 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Monday, May 9, 2022 11:36 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing issue on Windows
> 
> On Mon, 9 May 2022, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Martin Storsjö
> >> Sent: Sunday, May 8, 2022 10:02 PM
> >> To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> >> sharing issue on Windows
> >>
> >> On Sat, 7 May 2022, Soft Works wrote:
> >>
> >>>> This means that CRT objects (file descriptors from open(), FILE*
> >>>> opened
> >>>> with fopen/fdopen) mustn't be shared across DLLs; such an object
> >> must
> >>>> be
> >>>> opened, accessed and closed within the same DLL.
> >>>
> >>> This only happens when you explicitly modify the build
> configuration
> >>> to statically link to the CRT.
> >>
> >> No, this is not a custom build configuration. This is the build
> >> configuration you get if you configure with "configure --enable-
> shared
> >> --toolchain=msvc".
> >
> > Ok, then this is what needs to be fixed. When you configure for
> "shared",
> > the exe and dll binaries need to be all compiled with /MD.
> 
> I disagree. Both (statically linked or dynamically linked CRT) are
> entirely valid configurations, and ffmpeg works fine (except for this
> particular, so far marginally used, function) in both those build
> configurations.

I don't mean to change this as a measure for dealing with this issue 
and considering it fixed by that. Mixed CRT usage is not really the
typical and definitely not the recommended way to build a bunch of 
applications alongside a bunch of dynamic libraries.
That's why I think that this might be something to consider changing.

Besides that, having mixed runtimes and in general multiple versions
of dynamic libraries loaded in the same process is not only perfectly
valid, it's also one of the key reasons for the unparalleled backwards
compatibility that Windows provides in contrast to Unix based systems.
(what I mean is that a dll or application that was compiled 20 years
ago can still run or in case of a dll be loaded by a current application).


> >> Also, another fairly common situation where the "different CRTs"
> >> scenario
> >> happens if you'd e.g. build the ffmpeg libraries as DLLs with
> mingw,
> >> but
> >> then link against those DLLs with a user application built with
> MSVC.
> >
> > AFAIK, it is possible to create DLLs with mingw/MSYS2 in a way that
> > these can link to a specific version of the MS CRT, but that's just
> > a side note.
> 
> Yes, that's true. (As a side note to this side note, I'm the one who
> added
> support for UCRT in mingw-w64 in the first place, so I do know a thing
> or
> two about that.)

Cool :-)


> But in short, yes it's possible to spend effort at making them use the
> same shared CRT, but it's also fairly common to use different CRTs.

Yes, when you're mixing stuff from different sources, or just to name
one example: something like plugins.
But when you build an application alongside a set of libraries - that
is still a bit unusual, isn't it..

I don't actually care as I'm using the SMP project generator, it just
doesn't seem to be a good default IMO.

Kind regards,
softworkz


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-09 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Monday, May 9, 2022 12:47 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing issue on Windows
> 
> On Mon, 9 May 2022, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Martin Storsjö
> >> Sent: Monday, May 9, 2022 11:42 AM
> >> To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> >> sharing issue on Windows
> >>
> >> On Mon, 9 May 2022, Soft Works wrote:
> >>
> >>>
> >>>
> >>>> -Original Message-
> >>>> From: ffmpeg-devel  On Behalf Of
> >>>> Martin Storsjö
> >>>> Sent: Sunday, May 8, 2022 10:12 PM
> >>>> To: FFmpeg development discussions and patches  >>>> de...@ffmpeg.org>
> >>>> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT
> object
> >>>> sharing issue on Windows
> >>>>
> >>>> On Sat, 7 May 2022, Soft Works wrote:
> >>>>
> >>>>> Ah yes of course, thanks for the explanation. I still wonder
> >> whether
> >>>>> there aren't any other issues when multiple CRTs are being used?
> >>>>>
> >>>>> Or are the file IO APIs the only "weak" point with regards to
> >>>>> multiple CRTs being used?
> >>>>
> >>>> In the case of ffmpeg, yes.
> >>>>
> >>>> For generic library design, you'd have an issue anywhere where
> you
> >>>> pass
> >>>> CRT resources around - file descriptors from open, FILE*, and
> >> indeed
> >>>> as
> >>>> you mentioned - allocating and freeing memory with malloc/free in
> >>>> different DLLs. But as long as the library design is such that
> you
> >>>> don't
> >>>> hand over ownership of allocations and don't pass such objects
> >> across
> >>>> DLL
> >>>> boundaries, there's no issue.
> >>>
> >>> Yup, understood. I thought there would be many more, but I
> realized
> >>> that those "many more" I thought about are all C++ things, not C.
> >>>
> >>> So, putting this all together, I agree that the existence of
> >>> av_fopen_utf8 (as a public API!) is rather unfortunate. To make
> this
> >>> consistent, it would be necessary to provide av_ equivalents to
> >>> all the file APIs as well (but there are quite a few).
> >>
> >> Indeed - for the fopen family of functions, we would need to
> duplicate
> >> all
> >> of fopen/fclose/fprintf/fwrite/fputs and whatever might happen to
> be
> >> used.
> >> So that doesn't seem worthwhile.
> >>
> >>> So I wonder whether it wouldn't make sense to deprecate this as
> >>> a public API member?
> >>
> >> I agree that it probably would be the best way forward, to
> deprecate
> >> it as
> >> a public API, without any suggested replacement. A quick googling
> >> didn't
> >> find any real use of the function outside of ffmpeg itself, I only
> >> found
> >> hits in language wrappers (which try to map every single function
> to
> >> the
> >> other language). So I think that would have minimal impact on
> others.
> >>
> >> We could then adjust the function to be a header inline function
> >> (which
> >> takes care of the duplication into all libraries), just like the
> other
> >> wchar<->utf8 functions we have in libavutil/wchar_filename.h, so we
> >> safely
> >> could use them within fftools too.
> >
> > This would sound good to me, so when nobody objects, that would be
> > the way to go IMO.
> > And in case that somebody would object, the second best option could
> > be to deprecate it for Windows only (while api differences per
> platform
> > are surely not desirable, it might still be justified for a niche
> case
> > like this).
> >
> > BTW - could it be that your original patch missed to apply the same
> for
> > libavfilter?
> 
> At the time, there were no uses of the per-library duplicated
> functions
> within libavfilter, so I didn't add it there. If we add usage of them,

There are 9 uses already and my patch adds 6. 

> would indeed need to add the same duplication logic there too. (But if
> we
> make the function entirely inline in headers, as opposed to
> avpriv_open/ff_open, we don't need to do that for use of that
> function.)

Yup.

Thanks,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-09 Thread Martin Storsjö

On Mon, 9 May 2022, Soft Works wrote:


-Original Message-
From: ffmpeg-devel  On Behalf Of
Martin Storsjö
Sent: Monday, May 9, 2022 11:42 AM
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
sharing issue on Windows

On Mon, 9 May 2022, Soft Works wrote:





-Original Message-
From: ffmpeg-devel  On Behalf Of
Martin Storsjö
Sent: Sunday, May 8, 2022 10:12 PM
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
sharing issue on Windows

On Sat, 7 May 2022, Soft Works wrote:


Ah yes of course, thanks for the explanation. I still wonder

whether

there aren't any other issues when multiple CRTs are being used?

Or are the file IO APIs the only "weak" point with regards to
multiple CRTs being used?


In the case of ffmpeg, yes.

For generic library design, you'd have an issue anywhere where you
pass
CRT resources around - file descriptors from open, FILE*, and

indeed

as
you mentioned - allocating and freeing memory with malloc/free in
different DLLs. But as long as the library design is such that you
don't
hand over ownership of allocations and don't pass such objects

across

DLL
boundaries, there's no issue.


Yup, understood. I thought there would be many more, but I realized
that those "many more" I thought about are all C++ things, not C.

So, putting this all together, I agree that the existence of
av_fopen_utf8 (as a public API!) is rather unfortunate. To make this
consistent, it would be necessary to provide av_ equivalents to
all the file APIs as well (but there are quite a few).


Indeed - for the fopen family of functions, we would need to duplicate
all
of fopen/fclose/fprintf/fwrite/fputs and whatever might happen to be
used.
So that doesn't seem worthwhile.


So I wonder whether it wouldn't make sense to deprecate this as
a public API member?


I agree that it probably would be the best way forward, to deprecate
it as
a public API, without any suggested replacement. A quick googling
didn't
find any real use of the function outside of ffmpeg itself, I only
found
hits in language wrappers (which try to map every single function to
the
other language). So I think that would have minimal impact on others.

We could then adjust the function to be a header inline function
(which
takes care of the duplication into all libraries), just like the other
wchar<->utf8 functions we have in libavutil/wchar_filename.h, so we
safely
could use them within fftools too.


This would sound good to me, so when nobody objects, that would be
the way to go IMO.
And in case that somebody would object, the second best option could
be to deprecate it for Windows only (while api differences per platform
are surely not desirable, it might still be justified for a niche case
like this).

BTW - could it be that your original patch missed to apply the same for
libavfilter?


At the time, there were no uses of the per-library duplicated functions 
within libavfilter, so I didn't add it there. If we add usage of them, we 
would indeed need to add the same duplication logic there too. (But if we 
make the function entirely inline in headers, as opposed to 
avpriv_open/ff_open, we don't need to do that for use of that function.)


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-09 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Monday, May 9, 2022 11:42 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing issue on Windows
> 
> On Mon, 9 May 2022, Soft Works wrote:
> 
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Martin Storsjö
> >> Sent: Sunday, May 8, 2022 10:12 PM
> >> To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> >> sharing issue on Windows
> >>
> >> On Sat, 7 May 2022, Soft Works wrote:
> >>
> >>> Ah yes of course, thanks for the explanation. I still wonder
> whether
> >>> there aren't any other issues when multiple CRTs are being used?
> >>>
> >>> Or are the file IO APIs the only "weak" point with regards to
> >>> multiple CRTs being used?
> >>
> >> In the case of ffmpeg, yes.
> >>
> >> For generic library design, you'd have an issue anywhere where you
> >> pass
> >> CRT resources around - file descriptors from open, FILE*, and
> indeed
> >> as
> >> you mentioned - allocating and freeing memory with malloc/free in
> >> different DLLs. But as long as the library design is such that you
> >> don't
> >> hand over ownership of allocations and don't pass such objects
> across
> >> DLL
> >> boundaries, there's no issue.
> >
> > Yup, understood. I thought there would be many more, but I realized
> > that those "many more" I thought about are all C++ things, not C.
> >
> > So, putting this all together, I agree that the existence of
> > av_fopen_utf8 (as a public API!) is rather unfortunate. To make this
> > consistent, it would be necessary to provide av_ equivalents to
> > all the file APIs as well (but there are quite a few).
> 
> Indeed - for the fopen family of functions, we would need to duplicate
> all
> of fopen/fclose/fprintf/fwrite/fputs and whatever might happen to be
> used.
> So that doesn't seem worthwhile.
> 
> > So I wonder whether it wouldn't make sense to deprecate this as
> > a public API member?
> 
> I agree that it probably would be the best way forward, to deprecate
> it as
> a public API, without any suggested replacement. A quick googling
> didn't
> find any real use of the function outside of ffmpeg itself, I only
> found
> hits in language wrappers (which try to map every single function to
> the
> other language). So I think that would have minimal impact on others.
> 
> We could then adjust the function to be a header inline function
> (which
> takes care of the duplication into all libraries), just like the other
> wchar<->utf8 functions we have in libavutil/wchar_filename.h, so we
> safely
> could use them within fftools too.

This would sound good to me, so when nobody objects, that would be
the way to go IMO.
And in case that somebody would object, the second best option could
be to deprecate it for Windows only (while api differences per platform
are surely not desirable, it might still be justified for a niche case
like this). 

BTW - could it be that your original patch missed to apply the same for 
libavfilter?

Kind regards,
softworkz

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-09 Thread Martin Storsjö

On Mon, 9 May 2022, Soft Works wrote:





-Original Message-
From: ffmpeg-devel  On Behalf Of
Martin Storsjö
Sent: Sunday, May 8, 2022 10:12 PM
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
sharing issue on Windows

On Sat, 7 May 2022, Soft Works wrote:


Ah yes of course, thanks for the explanation. I still wonder whether
there aren't any other issues when multiple CRTs are being used?

Or are the file IO APIs the only "weak" point with regards to
multiple CRTs being used?


In the case of ffmpeg, yes.

For generic library design, you'd have an issue anywhere where you
pass
CRT resources around - file descriptors from open, FILE*, and indeed
as
you mentioned - allocating and freeing memory with malloc/free in
different DLLs. But as long as the library design is such that you
don't
hand over ownership of allocations and don't pass such objects across
DLL
boundaries, there's no issue.


Yup, understood. I thought there would be many more, but I realized
that those "many more" I thought about are all C++ things, not C.

So, putting this all together, I agree that the existence of
av_fopen_utf8 (as a public API!) is rather unfortunate. To make this
consistent, it would be necessary to provide av_ equivalents to
all the file APIs as well (but there are quite a few).


Indeed - for the fopen family of functions, we would need to duplicate all 
of fopen/fclose/fprintf/fwrite/fputs and whatever might happen to be used. 
So that doesn't seem worthwhile.



So I wonder whether it wouldn't make sense to deprecate this as
a public API member?


I agree that it probably would be the best way forward, to deprecate it as 
a public API, without any suggested replacement. A quick googling didn't 
find any real use of the function outside of ffmpeg itself, I only found 
hits in language wrappers (which try to map every single function to the 
other language). So I think that would have minimal impact on others.


We could then adjust the function to be a header inline function (which 
takes care of the duplication into all libraries), just like the other 
wchar<->utf8 functions we have in libavutil/wchar_filename.h, so we safely 
could use them within fftools too.


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-09 Thread Martin Storsjö

On Mon, 9 May 2022, Soft Works wrote:


-Original Message-
From: ffmpeg-devel  On Behalf Of
Martin Storsjö
Sent: Sunday, May 8, 2022 10:02 PM
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
sharing issue on Windows

On Sat, 7 May 2022, Soft Works wrote:


This means that CRT objects (file descriptors from open(), FILE*
opened
with fopen/fdopen) mustn't be shared across DLLs; such an object

must

be
opened, accessed and closed within the same DLL.


This only happens when you explicitly modify the build configuration
to statically link to the CRT.


No, this is not a custom build configuration. This is the build
configuration you get if you configure with "configure --enable-shared
--toolchain=msvc".


Ok, then this is what needs to be fixed. When you configure for "shared",
the exe and dll binaries need to be all compiled with /MD.


I disagree. Both (statically linked or dynamically linked CRT) are 
entirely valid configurations, and ffmpeg works fine (except for this 
particular, so far marginally used, function) in both those build 
configurations.



Also, another fairly common situation where the "different CRTs"
scenario
happens if you'd e.g. build the ffmpeg libraries as DLLs with mingw,
but
then link against those DLLs with a user application built with MSVC.


AFAIK, it is possible to create DLLs with mingw/MSYS2 in a way that
these can link to a specific version of the MS CRT, but that's just
a side note.


Yes, that's true. (As a side note to this side note, I'm the one who added 
support for UCRT in mingw-w64 in the first place, so I do know a thing or 
two about that.)


But in short, yes it's possible to spend effort at making them use the 
same shared CRT, but it's also fairly common to use different CRTs.


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-08 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Sunday, May 8, 2022 10:12 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing issue on Windows
> 
> On Sat, 7 May 2022, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Andreas Rheinhardt
> >> Sent: Saturday, May 7, 2022 6:32 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> >> sharing issue on Windows
> >>
> >> Soft Works:
> >>>
> >>>
> >>>> -Original Message-
> >>>
> >>> I don't have experience with that kind of setup, but I would have
> >>> thought that with separate CRTs, you could already get into
> trouble
> >>> when you would allocate a string in the main application which
> >>> you pass to any of the DLL's APIs and which might get freed by
> >>> the DLL at a later time - doesn't that fail?
> >>>
> >>
> >> Whenever any of the FFmpeg libraries takes ownership of a string or
> >> another buffer*, we require it to be freeable with av_free
> (typically
> >> by
> >> saying that it needs to be allocated with the av_malloc family of
> >> functions). So all allocs and frees have to happen in libavutil.
> This
> >> is
> >> also true for all the other allocations directly performed by the
> the
> >> FFmpeg libraries.
> >> (The only exceptions to this are AVBuffer(Ref)s which allow users
> to
> >> use
> >> custom allocators and destructors.)
> >
> > Ah yes of course, thanks for the explanation. I still wonder whether
> > there aren't any other issues when multiple CRTs are being used?
> >
> > Or are the file IO APIs the only "weak" point with regards to
> > multiple CRTs being used?
> 
> In the case of ffmpeg, yes.
> 
> For generic library design, you'd have an issue anywhere where you
> pass
> CRT resources around - file descriptors from open, FILE*, and indeed
> as
> you mentioned - allocating and freeing memory with malloc/free in
> different DLLs. But as long as the library design is such that you
> don't
> hand over ownership of allocations and don't pass such objects across
> DLL
> boundaries, there's no issue.

Yup, understood. I thought there would be many more, but I realized
that those "many more" I thought about are all C++ things, not C.

So, putting this all together, I agree that the existence of 
av_fopen_utf8 (as a public API!) is rather unfortunate. To make this 
consistent, it would be necessary to provide av_ equivalents to
all the file APIs as well (but there are quite a few).

So I wonder whether it wouldn't make sense to deprecate this as
a public API member?

Kind regards,
softworkz



___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-08 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Sunday, May 8, 2022 10:02 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing issue on Windows
> 
> On Sat, 7 May 2022, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Martin Storsjö
> >> Sent: Wednesday, April 20, 2022 2:48 PM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing
> >> issue on Windows
> >>
> >> Hi,
> >>
> >> I just became aware of the av_fopen_utf8 function - which was
> >> introduced
> >> to fix path name translations on Windows - actually has a notable
> >> design
> >> flaw.
> >
> > Hi Martin,
> >
> > I just became aware that somebody would be compiling ffmpeg like
> > this on Windows and I'm curious regarding the whereabouts..
> >
> >> Background:
> >>
> >> On Windows, a process can contain more than one C runtime (CRT);
> the
> >> system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC
> >> builds, each DLL/EXE can have one statically linked in instead of
> >> linking
> >> against a shared library CRT (and that's actually the default
> >> configuration when building with MSVC).
> >
> > The default configuration for both, EXE and DLL projects is to link
> > to the C runtime dynamically (crt dll).
> 
> No, that's not true. If you invoke e.g. "cl.exe myprog.c" without
> explicitly passing either -MD or -MT, the default is -MT, which is to
> statically link the CRT.

What I meant is when you create a new console project in Visual Studio,
the default is /MD, and the same is true when creating a new dll project.



> >> This means that CRT objects (file descriptors from open(), FILE*
> >> opened
> >> with fopen/fdopen) mustn't be shared across DLLs; such an object
> must
> >> be
> >> opened, accessed and closed within the same DLL.
> >
> > This only happens when you explicitly modify the build configuration
> > to statically link to the CRT.
> 
> No, this is not a custom build configuration. This is the build
> configuration you get if you configure with "configure --enable-shared
> --toolchain=msvc".

Ok, then this is what needs to be fixed. When you configure for "shared",
the exe and dll binaries need to be all compiled with /MD.


> > Why are you compiling it this way?
> > Your earlier patch is from 2013, so you seem to be doing so for
> > quite a while.
> 
> It's not that I'm shipping a production setup built this way. I just
> spent
> a fair amount of work to make ffmpeg work when built with MSVC; both
> built
> as static libraries and as DLLs. And I'd like to keep that working.
> Not
> only on the "seems to work for whatever is covered by fate" level, but
> also on the level of not using constructs that are known to not work.
> 
> As av_fopen_utf8 gets duplicated across the libraries by being in the
> same
> source file as the other functions, it works for all uses across the
> libraries, but doesn't work for uses outside of the libraries
> (fftools,
> external API users). That's why I'm hesitant against increasing the
> use of
> this function in fftools until we have resolve this issue.

Yes I agree to that. I will retract this part of my patchset.


> Also, another fairly common situation where the "different CRTs"
> scenario
> happens if you'd e.g. build the ffmpeg libraries as DLLs with mingw,
> but
> then link against those DLLs with a user application built with MSVC.

AFAIK, it is possible to create DLLs with mingw/MSYS2 in a way that
these can link to a specific version of the MS CRT, but that's just
a side note.

> > Is the file API the only case where you had any trouble?
> 
> As far as I remember, that was the only case of cross-library resource
> sharing issue I ran into at the time.


I'll follow up on this in your other reply.

Thanks,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-08 Thread Martin Storsjö

On Sat, 7 May 2022, Soft Works wrote:


-Original Message-
From: ffmpeg-devel  On Behalf Of
Andreas Rheinhardt
Sent: Saturday, May 7, 2022 6:32 AM
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
sharing issue on Windows

Soft Works:




-Original Message-


I don't have experience with that kind of setup, but I would have
thought that with separate CRTs, you could already get into trouble
when you would allocate a string in the main application which
you pass to any of the DLL's APIs and which might get freed by
the DLL at a later time - doesn't that fail?



Whenever any of the FFmpeg libraries takes ownership of a string or
another buffer*, we require it to be freeable with av_free (typically
by
saying that it needs to be allocated with the av_malloc family of
functions). So all allocs and frees have to happen in libavutil. This
is
also true for all the other allocations directly performed by the the
FFmpeg libraries.
(The only exceptions to this are AVBuffer(Ref)s which allow users to
use
custom allocators and destructors.)


Ah yes of course, thanks for the explanation. I still wonder whether
there aren't any other issues when multiple CRTs are being used?

Or are the file IO APIs the only "weak" point with regards to
multiple CRTs being used?


In the case of ffmpeg, yes.

For generic library design, you'd have an issue anywhere where you pass 
CRT resources around - file descriptors from open, FILE*, and indeed as 
you mentioned - allocating and freeing memory with malloc/free in 
different DLLs. But as long as the library design is such that you don't 
hand over ownership of allocations and don't pass such objects across DLL 
boundaries, there's no issue.


// Martin

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-08 Thread Martin Storsjö

On Sat, 7 May 2022, Soft Works wrote:


-Original Message-
From: ffmpeg-devel  On Behalf Of
Martin Storsjö
Sent: Wednesday, April 20, 2022 2:48 PM
To: ffmpeg-devel@ffmpeg.org
Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing
issue on Windows

Hi,

I just became aware of the av_fopen_utf8 function - which was
introduced
to fix path name translations on Windows - actually has a notable
design
flaw.


Hi Martin,

I just became aware that somebody would be compiling ffmpeg like
this on Windows and I'm curious regarding the whereabouts..


Background:

On Windows, a process can contain more than one C runtime (CRT); the
system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC
builds, each DLL/EXE can have one statically linked in instead of
linking
against a shared library CRT (and that's actually the default
configuration when building with MSVC).


The default configuration for both, EXE and DLL projects is to link
to the C runtime dynamically (crt dll).


No, that's not true. If you invoke e.g. "cl.exe myprog.c" without 
explicitly passing either -MD or -MT, the default is -MT, which is to 
statically link the CRT.



This means that CRT objects (file descriptors from open(), FILE*
opened
with fopen/fdopen) mustn't be shared across DLLs; such an object must
be
opened, accessed and closed within the same DLL.


This only happens when you explicitly modify the build configuration
to statically link to the CRT.


No, this is not a custom build configuration. This is the build 
configuration you get if you configure with "configure --enable-shared 
--toolchain=msvc".



Why are you compiling it this way?
Your earlier patch is from 2013, so you seem to be doing so for
quite a while.


It's not that I'm shipping a production setup built this way. I just spent 
a fair amount of work to make ffmpeg work when built with MSVC; both built 
as static libraries and as DLLs. And I'd like to keep that working. Not 
only on the "seems to work for whatever is covered by fate" level, but 
also on the level of not using constructs that are known to not work.


As av_fopen_utf8 gets duplicated across the libraries by being in the same 
source file as the other functions, it works for all uses across the 
libraries, but doesn't work for uses outside of the libraries (fftools, 
external API users). That's why I'm hesitant against increasing the use of 
this function in fftools until we have resolve this issue.


Also, another fairly common situation where the "different CRTs" scenario 
happens if you'd e.g. build the ffmpeg libraries as DLLs with mingw, but 
then link against those DLLs with a user application built with MSVC. Then 
it's not an issue with the function between the libraries, but it is an 
issue for use of av_fopen_utf8 in the user's code outside of libav*.



Is the file API the only case where you had any trouble?


As far as I remember, that was the only case of cross-library resource 
sharing issue I ran into at the time.


I don't have experience with that kind of setup, but I would have 
thought that with separate CRTs, you could already get into trouble when 
you would allocate a string in the main application which you pass to 
any of the DLL's APIs and which might get freed by the DLL at a later 
time - doesn't that fail?


No, it doesn't (as Andreas explained).

// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-06 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: Saturday, May 7, 2022 6:32 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing issue on Windows
> 
> Soft Works:
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Martin Storsjö
> >> Sent: Wednesday, April 20, 2022 2:48 PM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object
> sharing
> >> issue on Windows
> >>
> >> Hi,
> >>
> >> I just became aware of the av_fopen_utf8 function - which was
> >> introduced
> >> to fix path name translations on Windows - actually has a notable
> >> design
> >> flaw.
> >
> > Hi Martin,
> >
> > I just became aware that somebody would be compiling ffmpeg like
> > this on Windows and I'm curious regarding the whereabouts..
> >
> >> Background:
> >>
> >> On Windows, a process can contain more than one C runtime (CRT);
> the
> >> system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC
> >> builds, each DLL/EXE can have one statically linked in instead of
> >> linking
> >> against a shared library CRT (and that's actually the default
> >> configuration when building with MSVC).
> >
> > The default configuration for both, EXE and DLL projects is to link
> > to the C runtime dynamically (crt dll).
> >
> >> This means that CRT objects (file descriptors from open(), FILE*
> >> opened
> >> with fopen/fdopen) mustn't be shared across DLLs; such an object
> must
> >> be
> >> opened, accessed and closed within the same DLL.
> >
> > This only happens when you explicitly modify the build configuration
> > to statically link to the CRT.
> > It is generally discouraged to mix (or have multiple) CRTs in a
> single
> > process, but it's surely valid and there can be very good reasons to
> > do so. Yet, such reasons are typically about achieving a certain
> level
> > of independence between libraries and their dependencies and
> > interdependencies.
> > What's probably a bit more unusual is to build libraries like the
> > ffmpeg libs which are very closely related and dependent in a way
> > that each of them has its own static copy of the CRT compiled into
> it.
> >
> > I'm curious about two things:
> >
> > Why are you compiling it this way?
> > Your earlier patch is from 2013, so you seem to be doing so for
> > quite a while.
> >
> >
> > Is the file API the only case where you had any trouble?
> >
> > I don't have experience with that kind of setup, but I would have
> > thought that with separate CRTs, you could already get into trouble
> > when you would allocate a string in the main application which
> > you pass to any of the DLL's APIs and which might get freed by
> > the DLL at a later time - doesn't that fail?
> >
> 
> Whenever any of the FFmpeg libraries takes ownership of a string or
> another buffer*, we require it to be freeable with av_free (typically
> by
> saying that it needs to be allocated with the av_malloc family of
> functions). So all allocs and frees have to happen in libavutil. This
> is
> also true for all the other allocations directly performed by the the
> FFmpeg libraries.
> (The only exceptions to this are AVBuffer(Ref)s which allow users to
> use
> custom allocators and destructors.)

Ah yes of course, thanks for the explanation. I still wonder whether
there aren't any other issues when multiple CRTs are being used?

Or are the file IO APIs the only "weak" point with regards to 
multiple CRTs being used?

Thanks,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-06 Thread Andreas Rheinhardt
Soft Works:
> 
> 
>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
>> Martin Storsjö
>> Sent: Wednesday, April 20, 2022 2:48 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing
>> issue on Windows
>>
>> Hi,
>>
>> I just became aware of the av_fopen_utf8 function - which was
>> introduced
>> to fix path name translations on Windows - actually has a notable
>> design
>> flaw.
> 
> Hi Martin,
> 
> I just became aware that somebody would be compiling ffmpeg like 
> this on Windows and I'm curious regarding the whereabouts..
> 
>> Background:
>>
>> On Windows, a process can contain more than one C runtime (CRT); the
>> system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC
>> builds, each DLL/EXE can have one statically linked in instead of
>> linking
>> against a shared library CRT (and that's actually the default
>> configuration when building with MSVC).
> 
> The default configuration for both, EXE and DLL projects is to link
> to the C runtime dynamically (crt dll).
> 
>> This means that CRT objects (file descriptors from open(), FILE*
>> opened
>> with fopen/fdopen) mustn't be shared across DLLs; such an object must
>> be
>> opened, accessed and closed within the same DLL.
> 
> This only happens when you explicitly modify the build configuration
> to statically link to the CRT. 
> It is generally discouraged to mix (or have multiple) CRTs in a single
> process, but it's surely valid and there can be very good reasons to
> do so. Yet, such reasons are typically about achieving a certain level
> of independence between libraries and their dependencies and 
> interdependencies.
> What's probably a bit more unusual is to build libraries like the
> ffmpeg libs which are very closely related and dependent in a way
> that each of them has its own static copy of the CRT compiled into it.
> 
> I'm curious about two things:
> 
> Why are you compiling it this way?
> Your earlier patch is from 2013, so you seem to be doing so for
> quite a while.
> 
> 
> Is the file API the only case where you had any trouble?
> 
> I don't have experience with that kind of setup, but I would have
> thought that with separate CRTs, you could already get into trouble
> when you would allocate a string in the main application which 
> you pass to any of the DLL's APIs and which might get freed by
> the DLL at a later time - doesn't that fail?
> 

Whenever any of the FFmpeg libraries takes ownership of a string or
another buffer*, we require it to be freeable with av_free (typically by
saying that it needs to be allocated with the av_malloc family of
functions). So all allocs and frees have to happen in libavutil. This is
also true for all the other allocations directly performed by the the
FFmpeg libraries.
(The only exceptions to this are AVBuffer(Ref)s which allow users to use
custom allocators and destructors.)

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-05-06 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Wednesday, April 20, 2022 2:48 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing
> issue on Windows
> 
> Hi,
> 
> I just became aware of the av_fopen_utf8 function - which was
> introduced
> to fix path name translations on Windows - actually has a notable
> design
> flaw.

Hi Martin,

I just became aware that somebody would be compiling ffmpeg like 
this on Windows and I'm curious regarding the whereabouts..

> Background:
> 
> On Windows, a process can contain more than one C runtime (CRT); the
> system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC
> builds, each DLL/EXE can have one statically linked in instead of
> linking
> against a shared library CRT (and that's actually the default
> configuration when building with MSVC).

The default configuration for both, EXE and DLL projects is to link
to the C runtime dynamically (crt dll).

> This means that CRT objects (file descriptors from open(), FILE*
> opened
> with fopen/fdopen) mustn't be shared across DLLs; such an object must
> be
> opened, accessed and closed within the same DLL.

This only happens when you explicitly modify the build configuration
to statically link to the CRT. 
It is generally discouraged to mix (or have multiple) CRTs in a single
process, but it's surely valid and there can be very good reasons to
do so. Yet, such reasons are typically about achieving a certain level
of independence between libraries and their dependencies and 
interdependencies.
What's probably a bit more unusual is to build libraries like the
ffmpeg libs which are very closely related and dependent in a way
that each of them has its own static copy of the CRT compiled into it.

I'm curious about two things:

Why are you compiling it this way?
Your earlier patch is from 2013, so you seem to be doing so for
quite a while.


Is the file API the only case where you had any trouble?

I don't have experience with that kind of setup, but I would have
thought that with separate CRTs, you could already get into trouble
when you would allocate a string in the main application which 
you pass to any of the DLL's APIs and which might get freed by
the DLL at a later time - doesn't that fail?

Kind regards,
softworkz

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] av_fopen_utf8 and cross-DLL CRT object sharing issue on Windows

2022-04-20 Thread Martin Storsjö

Hi,

I just became aware of the av_fopen_utf8 function - which was introduced 
to fix path name translations on Windows - actually has a notable design 
flaw.



Background:

On Windows, a process can contain more than one C runtime (CRT); the 
system comes with two shared ones (UCRT and msvcrt.dll) and in MSVC 
builds, each DLL/EXE can have one statically linked in instead of linking 
against a shared library CRT (and that's actually the default 
configuration when building with MSVC).


This means that CRT objects (file descriptors from open(), FILE* opened 
with fopen/fdopen) mustn't be shared across DLLs; such an object must be 
opened, accessed and closed within the same DLL.



The issue:

This was fixed for the avpriv_open() function in 
e743e7ae6ee7e535c4394bec6fe6650d2b0dbf65 by duplicating the file_open.c 
source file in all libav* libraries (in MSVC builds, and renaming 
avpriv_open to ff_open), so that each library gets their own copy of 
ff_open (which isn't exported), so that all calls to open/read/write/close 
are done within the same DLL.


When av_fopen_utf8 was added afterwards, in 
85cabf1ca98fcc502fcf5b8d6bfb6d8061c2caef, this issue wasn't taken into 
account - although the issue is somewhat eased by lucky coincidence.


As av_fopen_utf8 is implemented in the same source file, 
libavutil/file_open.c, which gets duplicated in all libraries that use it, 
all uses of the function in other libraries (such as libavformat) actually 
end up using their own copy of it. (This also means that all the libav* 
DLLs export this function.) But for any users of the function outside of 
the ffmpeg libraries, this function (in libavutil, or whichever library it 
ends up imported from) returns a FILE* allocated by libavutil's CRT, which 
then can't be used with the fread/fwrite/fclose/whatever functions in the 
DLL/EXE that called it.


One concrete example of this is that the function is used for the twopass 
log file in fftools/ffmpeg_opt.c. To see the issue in action, build ffmpeg 
with MSVC with this config:


../ffmpeg/configure --enable-shared --toolchain=msvc --prefix=

Then try to do a twopass encoding with it:

ffmpeg -i  -an -c:v ffv1 -pass 1 -f null -

ffmpeg -i  -an -c:v ffv1 -pass 2 -y test-2pass.mkv

The same issue would appear anywhere this function is used from libavutil 
built as a DLL, from a caller built with a different CRT choice (e.g. 
often if mixing mingw/MSVC builds, which otherwise is supported just 
fine). (If built with a shared CRT, i.e. configured with 
--extra-cflags=-MD, it does work though.)



As this is a public function, we can't really do many tricks like we do 
within the libraries. (On the other hand, while it is a public function, 
it doesn't seem to be used much outside of ffmpeg, other than in ffmpeg 
API bindings for other languages.)


I guess the only really robust solution would be to turn av_fopen_utf8 
into a static inline function within the headers, essentially inlineing a 
copy of wchar_filename.h there, so that it expands to a call to the 
callers' _wfopen or similar. But that would end up polluting users' code 
by implicitly including windows.h everywhere, which really isn't nice to 
do either.


Or should we just document this issue, discourage further external use of 
the function, and fold this function into a ffmpeg-internal inline helper 
like libavutil/whcar_filename.h?



// Martin

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".