[FFmpeg-devel] [PATCH 1/3] fftools: Stop using av_fopen_utf8

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Tuesday, May 24, 2022 10:22 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/3] fftools: Stop using av_fopen_utf8
> 
> On Tue, 24 May 2022, Soft Works wrote:
> 
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Martin
> >> Storsjö
> >> Sent: Tuesday, May 24, 2022 11:29 AM
> >> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH 1/3] fftools: Stop using
> av_fopen_utf8
> >>
> >> On Mon, 23 May 2022, Soft Works wrote:
> >>
> >>> Great. I rebased and resubmitted both patchsets. The primary long-path
> >>> patchset didn't need any change.
> >>>
> >>> Considerations for the latter were:
> >>>
> >>> - Should the file wchar_filename.h be renamed as it is now containing
> >>>  the path prefixing code?
> >>
> >> I guess we could do that later at some point, but I don't see it as
> >> urgent.
> >>
> >>> - I have kept the path functions in the same way like .NET does it,
> >>>  just for easy reference and following. Compilers will inline
> >>>  them anyway (my pov). Maybe others don't like that. I can change
> >>>  if it's got to be.
> >>
> >> Having the individual functions inline compared to merging it all in
> one
> >> big function doesn't matter to me. But the amount of code in this
> inline
> >> header is growing a bit big, to the point that I think it is measurable
> >> when multiple files within the same library use these functions. Longer
> >> term, it would probably make sense to make them more shared in some
> way.
> >
> >> If C would have the C++ style deduplication feature for non-static
> inline
> >> functions, this wouldn't be an issue. One could consider making them
> ff_
> >> functions and carry a copy in each library too, maybe. (But that also
> >> makes it trickier to use in fftools.)
> >
> > A copy in each library - isn't that exactly what's already happening?
> >
> > The get_extended_win32_path() is used from 2 places:
> >
> > 2. os_support.h
> >
> > This is used in libavformat exclusively. But in this case, the code gets
> > duplicated actually for each consumption of one of those file functions.
> > There aren't many usages in total, though, and I don't see any trend
> > on the horizon for sudden increase, so I agree that there's no urgency.
> 
> Yes, this is what I was referring to. It's clearly more than one use. When
> counting files that use mkdir, unlink or "struct stat", I find around 9
> individual .c files in libavformat, that would end up with static inline
> copies of all of this.
> 
> Compared with the av_fopen_utf8 case, I first tested fixing it by making
> it a static inline function in a libavutil header, but that did increase
> the size of a built DLL by a couple KB. I guess this increases it by a
> bit more than that.
> 
> It's still not a lot, and this isn't a blocker (and I probably prefer that
> we don't try to redesign this issue here now, in order not to drag out the
> review further)

I'm glad you're saying that ;-)

It probably won't be difficult to improve this in a future change, making it
a wchar_filename.c plus wchar_filename.h file in libavutil. If I'm not 
mistaken, there shouldn't be an issue with multiple CRTs as memory is 
managed by av_ functions?

Thanks,
sw