[FFmpeg-devel,v5,2/2] avformat/os_support: Support long file names on Windows

From: softworkz <softworkz@hotmail.com>

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavformat/os_support.h | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

On Tue, 24 May 2022, softworkz wrote:

> From: softworkz <softworkz@hotmail.com>
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
> libavformat/os_support.h | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/os_support.h b/libavformat/os_support.h
> index 5e6b32d2dc..d4c07803a5 100644
> --- a/libavformat/os_support.h
> +++ b/libavformat/os_support.h
> @@ -49,7 +49,13 @@
> #  ifdef stat
> #   undef stat
> #  endif
> -#  define stat _stati64
> +#  define stat win32_stat
> +
> +    struct win32_stat
> +    {
> +        struct _stati64;
> +    };

Is it possible to work around this issue by doing "#define stat(a,b)" 
which only should apply on the function, not to the struct? Then we can't 
redirect "struct stat" into "struct _stati64" at the same time...

A safe way forward could be to switch code to just using "struct 
ff_stat_struct", and define ff_stat_struct to the name of the struct we 
exepct to use. It's not pretty, and it affects users which no longer can 
use the default POSIX stat form of the calls, but it would fix the issue 
of redirecting the struct and function separately, without needing to know 
what exactly is in the struct (because we really shouldn't be 
hardcoding/assuming that).

// Martin

> -----Original Message-----
> From: Martin Storsjö <martin@martin.st>
> Sent: Tuesday, May 24, 2022 11:23 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: softworkz <softworkz@hotmail.com>; Hendrik Leppkes
> <h.leppkes@gmail.com>
> Subject: Re: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> long file names on Windows
> 
> On Tue, 24 May 2022, softworkz wrote:
> 
> > From: softworkz <softworkz@hotmail.com>
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> > libavformat/os_support.h | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavformat/os_support.h b/libavformat/os_support.h
> > index 5e6b32d2dc..d4c07803a5 100644
> > --- a/libavformat/os_support.h
> > +++ b/libavformat/os_support.h
> > @@ -49,7 +49,13 @@
> > #  ifdef stat
> > #   undef stat
> > #  endif
> > -#  define stat _stati64
> > +#  define stat win32_stat
> > +
> > +    struct win32_stat
> > +    {
> > +        struct _stati64;
> > +    };
> 
> Is it possible to work around this issue by doing "#define stat(a,b)"
> which only should apply on the function, not to the struct? 

How could this be possible? A define is only doing string replacements,
so I wouldn't know how it could be restricted to the function, but 
not the struct.

> A safe way forward could be to switch code to just using "struct
> ff_stat_struct", and define ff_stat_struct to the name of the struct we
> exepct to use. It's not pretty, and it affects users which no longer can
> use the default POSIX stat form of the calls

That's why I took the effort to make this work.

> but it would fix the issue
> of redirecting the struct and function separately, without needing to know
> what exactly is in the struct (because we really shouldn't be
> hardcoding/assuming that).

That doesn't apply because the current code already does this:

DEF_FS_FUNCTION2(stat, _wstati64, _stati64, struct stat*)

Which means that it specifically chooses _stati64 which is a public
MS API: 

https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170

And we know that it takes _stati64 as the parameter.

This code:

    struct win32_stat
    {
        struct _stati64;
    };

makes use of a MS compiler feature called "anonymous structures":

https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2013/z2cx9y4f(v=vs.120)?redirectedfrom=MSDN

This way, we automatically "inherit" the members of the struct
(whatever their members would be).


Thanks for reviewing,
softworkz

On Tue, 24 May 2022, Soft Works wrote:

>> -----Original Message-----
>> From: Martin Storsjö <martin@martin.st>
>> Sent: Tuesday, May 24, 2022 11:23 AM
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Cc: softworkz <softworkz@hotmail.com>; Hendrik Leppkes
>> <h.leppkes@gmail.com>
>> Subject: Re: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
>> long file names on Windows
>>
>> On Tue, 24 May 2022, softworkz wrote:
>>
>>> From: softworkz <softworkz@hotmail.com>
>>>
>>> Signed-off-by: softworkz <softworkz@hotmail.com>
>>> ---
>>> libavformat/os_support.h | 16 +++++++++++-----
>>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavformat/os_support.h b/libavformat/os_support.h
>>> index 5e6b32d2dc..d4c07803a5 100644
>>> --- a/libavformat/os_support.h
>>> +++ b/libavformat/os_support.h
>>> @@ -49,7 +49,13 @@
>>> #  ifdef stat
>>> #   undef stat
>>> #  endif
>>> -#  define stat _stati64
>>> +#  define stat win32_stat
>>> +
>>> +    struct win32_stat
>>> +    {
>>> +        struct _stati64;
>>> +    };
>>
>> Is it possible to work around this issue by doing "#define stat(a,b)"
>> which only should apply on the function, not to the struct?
>
> How could this be possible? A define is only doing string replacements,
> so I wouldn't know how it could be restricted to the function, but
> not the struct.

If unsure about a tool feature, please try it out for yourself. Yes, a 
define is only a string replacement, but a define with parameters only 
matches the string occurs with parenthesis afterwards. Consider this 
example:

$ cat test.c
#define stat(a, b) win32_stat(a, b)

struct stat {
 	int a, b, c;
};

void stat (struct stat *st, const char* filename);

void func(const char* filename) {
 	struct stat st;
 	stat (&st, filename);
}
$ cc -E test.c
# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 31 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "<command-line>" 2
# 1 "test.c"


struct stat {
  int a, b, c;
};

void win32_stat(struct stat *st, const char* filename);

void func(const char* filename) {
  struct stat st;
  win32_stat(&st, filename);
}


So here, the stat -> win32_stat rewrite only applied on the function 
declaration and call, but not on the structs.

Not saying that this necessarily is the way forward, but I was just 
mentioning it as a potential option to consider.

>> A safe way forward could be to switch code to just using "struct
>> ff_stat_struct", and define ff_stat_struct to the name of the struct we
>> exepct to use. It's not pretty, and it affects users which no longer can
>> use the default POSIX stat form of the calls
>
> That's why I took the effort to make this work.
>
>> but it would fix the issue
>> of redirecting the struct and function separately, without needing to know
>> what exactly is in the struct (because we really shouldn't be
>> hardcoding/assuming that).
>
> That doesn't apply because the current code already does this:
>
> DEF_FS_FUNCTION2(stat, _wstati64, _stati64, struct stat*)
>
> Which means that it specifically chooses _stati64 which is a public
> MS API:
>
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?view=msvc-170
>
> And we know that it takes _stati64 as the parameter.

Yes, there is no disagreement about that part.

> This code:
>
>    struct win32_stat
>    {
>        struct _stati64;
>    };
>
> makes use of a MS compiler feature called "anonymous structures":
>
> https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2013/z2cx9y4f(v=vs.120)?redirectedfrom=MSDN
>
> This way, we automatically "inherit" the members of the struct
> (whatever their members would be).

This, as the article itself clearly declares, is a C language extension. 
GCC allows it in mingw mode, but Clang doesn't. (It's possible to use it 
in Clang too if you enable it with -fms-extensions though.)

$ cat test2.c
struct orig_struct {
 	int a, b, c;
};
struct my_struct {
 	struct orig_struct;
};
void set(struct my_struct *st) {
 	st->a = 42;
}
$ clang -target x86_64-w64-mingw32 -c test2.c
test2.c:5:2: warning: declaration does not declare anything [-Wmissing-declarations]
         struct orig_struct;
         ^
test2.c:8:6: error: no member named 'a' in 'struct my_struct'
         st->a = 42;
         ~~  ^
1 warning and 1 error generated.
$ clang -target x86_64-w64-mingw32 -c test2.c -fms-extensions
test2.c:5:2: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag]
         struct orig_struct;
         ^
1 warning generated.


// Martin

> -----Original Message-----
> From: Martin Storsjö <martin@martin.st>
> Sent: Tuesday, May 24, 2022 12:26 PM
> To: Soft Works <softworkz@hotmail.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>;
> Hendrik Leppkes <h.leppkes@gmail.com>
> Subject: RE: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> long file names on Windows
> 
> On Tue, 24 May 2022, Soft Works wrote:
> 
> >> -----Original Message-----
> >> From: Martin Storsjö <martin@martin.st>
> >> Sent: Tuesday, May 24, 2022 11:23 AM
> >> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> >> Cc: softworkz <softworkz@hotmail.com>; Hendrik Leppkes
> >> <h.leppkes@gmail.com>
> >> Subject: Re: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> >> long file names on Windows
> >>
> >> On Tue, 24 May 2022, softworkz wrote:
> >>
> >>> From: softworkz <softworkz@hotmail.com>
> >>>
> >>> Signed-off-by: softworkz <softworkz@hotmail.com>
> >>> ---
> >>> libavformat/os_support.h | 16 +++++++++++-----
> >>> 1 file changed, 11 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/libavformat/os_support.h b/libavformat/os_support.h
> >>> index 5e6b32d2dc..d4c07803a5 100644
> >>> --- a/libavformat/os_support.h
> >>> +++ b/libavformat/os_support.h
> >>> @@ -49,7 +49,13 @@
> >>> #  ifdef stat
> >>> #   undef stat
> >>> #  endif
> >>> -#  define stat _stati64
> >>> +#  define stat win32_stat
> >>> +
> >>> +    struct win32_stat
> >>> +    {
> >>> +        struct _stati64;
> >>> +    };
> >>
> >> Is it possible to work around this issue by doing "#define stat(a,b)"
> >> which only should apply on the function, not to the struct?
> >
> > How could this be possible? A define is only doing string replacements,
> > so I wouldn't know how it could be restricted to the function, but
> > not the struct.
> 
> If unsure about a tool feature, please try it out for yourself.

I did :-)
(very extensively in fact)

> Yes, a
> define is only a string replacement, but a define with parameters only
> matches the string occurs with parenthesis afterwards. 

Yes, that's true, but we need to rename both, the function and the
struct, not just the function.


Your example doesn't quite match the situation.

// Let's start with the pre-requisites which cannot be changed
// Posix definitions (stat.h)
struct stat {
 	int a, b, c;
};

// This is the regular definition
int stat(char *fileName, struct stat *par)

// With your define, we replace the function, but not the 
// struct. So we would call the WinAPI function with 
// the wrong struct.
#define stat(a, b) win32_stat(a, b)

// I don't know why you have this in your example at
// this place. It comes from some include file and
// it is not the one we need to use.
// if you mean to redefine it here: that was my previous
// approach. You need to copy the struct (but rename it)
struct stat {
 	int a, b, c;
};

// ----------------- other example:

// This function needs to call the windows function,
// so we cannot use the regular stat struct as parameter
void win32_stat(struct stat *st, const char* filename);



> So here, the stat -> win32_stat rewrite only applied on the function
> declaration and call, but not on the structs.

Exactly, but the struct must be rewritten as well and this is not
possible. 

Neither this:

#define stat _stati64
#define stat(a, b) win32_stat(a, b)

nor this:

#define stat(a, b) win32_stat(a, b)
#define stat _stati64

is working (yes, I tried ;-)

> This, as the article itself clearly declares, is a C language extension.
> GCC allows it in mingw mode

Yes I had read about that.

> but Clang doesn't. (It's possible to use it
> in Clang too if you enable it with -fms-extensions though.)

Is it possible to compile ffmpeg for Windows using Clang?
And if yes, does it even work without that flag?
(assuming it was introduced in order to be able to 
compile Windows stuff).


Thanks again,
sw

On Tue, 24 May 2022, Soft Works wrote:

>> -----Original Message-----
>> From: Martin Storsjö <martin@martin.st>
>> Sent: Tuesday, May 24, 2022 12:26 PM
>> To: Soft Works <softworkz@hotmail.com>
>> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>;
>> Hendrik Leppkes <h.leppkes@gmail.com>
>> Subject: RE: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
>> long file names on Windows
>>
>> On Tue, 24 May 2022, Soft Works wrote:
>>
>>>> -----Original Message-----
>>>> From: Martin Storsjö <martin@martin.st>
>>>> Sent: Tuesday, May 24, 2022 11:23 AM
>>>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@ffmpeg.org>
>>>> Cc: softworkz <softworkz@hotmail.com>; Hendrik Leppkes
>>>> <h.leppkes@gmail.com>
>>>> Subject: Re: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
>>>> long file names on Windows
>>>>
>>>> On Tue, 24 May 2022, softworkz wrote:
>>>>
>>>>> From: softworkz <softworkz@hotmail.com>
>>>>>
>>>>> Signed-off-by: softworkz <softworkz@hotmail.com>
>>>>> ---
>>>>> libavformat/os_support.h | 16 +++++++++++-----
>>>>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/os_support.h b/libavformat/os_support.h
>>>>> index 5e6b32d2dc..d4c07803a5 100644
>>>>> --- a/libavformat/os_support.h
>>>>> +++ b/libavformat/os_support.h
>>>>> @@ -49,7 +49,13 @@
>>>>> #  ifdef stat
>>>>> #   undef stat
>>>>> #  endif
>>>>> -#  define stat _stati64
>>>>> +#  define stat win32_stat
>>>>> +
>>>>> +    struct win32_stat
>>>>> +    {
>>>>> +        struct _stati64;
>>>>> +    };
>>>>
>>>> Is it possible to work around this issue by doing "#define stat(a,b)"
>>>> which only should apply on the function, not to the struct?
>>>
>>> How could this be possible? A define is only doing string replacements,
>>> so I wouldn't know how it could be restricted to the function, but
>>> not the struct.
>>
>> If unsure about a tool feature, please try it out for yourself.
>
> I did :-)
> (very extensively in fact)
>
>> Yes, a
>> define is only a string replacement, but a define with parameters only
>> matches the string occurs with parenthesis afterwards.
>
> Yes, that's true, but we need to rename both, the function and the
> struct, not just the function.


I know. But you said:

> How could this be possible? A define is only doing string replacements, 
> so I wouldn't know how it could be restricted to the function, but not 
> the struct.

And I showed how a define can apply to only one but not the other. Which 
seemed to be news to in your prior mail.

Note how I also said:

"Not saying that this necessarily is the way forward, but I was just 
mentioning it as a potential option to consider."

> Your example doesn't quite match the situation.

Yes I know.

I just brought it up as a possibly thing for discussion, and you derailed 
it by discussing whether it even works. Yes it works, but after looking 
more into it, I agree that it probably won't help in this situation.

>> but Clang doesn't. (It's possible to use it
>> in Clang too if you enable it with -fms-extensions though.)
>
> Is it possible to compile ffmpeg for Windows using Clang?
> And if yes, does it even work without that flag?
> (assuming it was introduced in order to be able to
> compile Windows stuff).

Yes, it is possible to build it with Clang without any custom extra flags 
to enable nondefault modes. In fact, it's tested continuously on FATE too:

http://fate.ffmpeg.org/history.cgi?slot=x86_64-mingw32-clang-trunk

Also for other architectures, e.g.:

http://fate.ffmpeg.org/history.cgi?slot=aarch64-mingw32-clang-trunk

// Martin

> -----Original Message-----
> From: Martin Storsjö <martin@martin.st>
> Sent: Tuesday, May 24, 2022 1:26 PM
> To: Soft Works <softworkz@hotmail.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>;
> Hendrik Leppkes <h.leppkes@gmail.com>
> Subject: RE: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> long file names on Windows
> 
> On Tue, 24 May 2022, Soft Works wrote:
> 
> >> -----Original Message-----
> >> From: Martin Storsjö <martin@martin.st>
> >> Sent: Tuesday, May 24, 2022 12:26 PM
> >> To: Soft Works <softworkz@hotmail.com>
> >> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>;
> >> Hendrik Leppkes <h.leppkes@gmail.com>
> >> Subject: RE: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> >> long file names on Windows
> >>
> >> On Tue, 24 May 2022, Soft Works wrote:
> >>
> >>>> -----Original Message-----
> >>>> From: Martin Storsjö <martin@martin.st>
> >>>> Sent: Tuesday, May 24, 2022 11:23 AM
> >>>> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel@ffmpeg.org>
> >>>> Cc: softworkz <softworkz@hotmail.com>; Hendrik Leppkes
> >>>> <h.leppkes@gmail.com>
> >>>> Subject: Re: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support:
> Support
> >>>> long file names on Windows
> >>>>
> >>>> On Tue, 24 May 2022, softworkz wrote:
> >>>>
> >>>>> From: softworkz <softworkz@hotmail.com>
> >>>>>
> >>>>> Signed-off-by: softworkz <softworkz@hotmail.com>
> >>>>> ---
> >>>>> libavformat/os_support.h | 16 +++++++++++-----
> >>>>> 1 file changed, 11 insertions(+), 5 deletions(-)
> >>>>>
> >>>>> diff --git a/libavformat/os_support.h b/libavformat/os_support.h
> >>>>> index 5e6b32d2dc..d4c07803a5 100644
> >>>>> --- a/libavformat/os_support.h
> >>>>> +++ b/libavformat/os_support.h
> >>>>> @@ -49,7 +49,13 @@
> >>>>> #  ifdef stat
> >>>>> #   undef stat
> >>>>> #  endif
> >>>>> -#  define stat _stati64
> >>>>> +#  define stat win32_stat
> >>>>> +
> >>>>> +    struct win32_stat
> >>>>> +    {
> >>>>> +        struct _stati64;
> >>>>> +    };
> >>>>
> >>>> Is it possible to work around this issue by doing "#define stat(a,b)"
> >>>> which only should apply on the function, not to the struct?
> >>>
> >>> How could this be possible? A define is only doing string
> replacements,
> >>> so I wouldn't know how it could be restricted to the function, but
> >>> not the struct.
> >>
> >> If unsure about a tool feature, please try it out for yourself.
> >
> > I did :-)
> > (very extensively in fact)
> >
> >> Yes, a
> >> define is only a string replacement, but a define with parameters only
> >> matches the string occurs with parenthesis afterwards.
> >
> > Yes, that's true, but we need to rename both, the function and the
> > struct, not just the function.
> 
> 
> I know. But you said:
> 
> > How could this be possible? A define is only doing string replacements,
> > so I wouldn't know how it could be restricted to the function, but not
> > the struct.
> 
> And I showed how a define can apply to only one but not the other. Which
> seemed to be news to in your prior mail.

Alright yea - thanks for pointing this out. I knew about this kind of
macros, but I hadn't taken them into account in this context because
all my attempts were focusing on the struct side and getting this
separated with typedef and #undef and those things, so it was indeed
opening up a dimension I hadn't considered.
Thanks for the example!


> Note how I also said:
> 
> "Not saying that this necessarily is the way forward, but I was just
> mentioning it as a potential option to consider."
> 
> > Your example doesn't quite match the situation.
> 
> Yes I know.

Sorry, I had understood your message in a way that you would be saying
that it would be working and just not a nice solution.


> I just brought it up as a possibly thing for discussion, and you derailed
> it by discussing whether it even works. 

Nah - I meant whether it works for the given situation, I had no doubt
that your example is working as-is.

 
> >> but Clang doesn't. (It's possible to use it
> >> in Clang too if you enable it with -fms-extensions though.)
> >
> > Is it possible to compile ffmpeg for Windows using Clang?
> > And if yes, does it even work without that flag?
> > (assuming it was introduced in order to be able to
> > compile Windows stuff).
> 
> Yes, it is possible to build it with Clang without any custom extra flags
> to enable nondefault modes. In fact, it's tested continuously on FATE too:
> 
> http://fate.ffmpeg.org/history.cgi?slot=x86_64-mingw32-clang-trunk
> 
> Also for other architectures, e.g.:
> 
> http://fate.ffmpeg.org/history.cgi?slot=aarch64-mingw32-clang-trunk


OK, thanks for the pointers. I'm not sure whether it would be 
acceptable to require this compilation flag for Windows builds?

Can you think of any other ideas?

Thank you very much,
softworkz

On Tue, 24 May 2022, Soft Works wrote:

>> -----Original Message-----
>> From: Martin Storsjö <martin@martin.st>
>> Sent: Tuesday, May 24, 2022 1:26 PM
>> To: Soft Works <softworkz@hotmail.com>
>> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>;
>> Hendrik Leppkes <h.leppkes@gmail.com>
>> Subject: RE: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
>> long file names on Windows
>>
>> On Tue, 24 May 2022, Soft Works wrote:
>>
>>>> but Clang doesn't. (It's possible to use it
>>>> in Clang too if you enable it with -fms-extensions though.)
>>>
>>> Is it possible to compile ffmpeg for Windows using Clang?
>>> And if yes, does it even work without that flag?
>>> (assuming it was introduced in order to be able to
>>> compile Windows stuff).
>>
>> Yes, it is possible to build it with Clang without any custom extra flags
>> to enable nondefault modes. In fact, it's tested continuously on FATE too:
>>
>> http://fate.ffmpeg.org/history.cgi?slot=x86_64-mingw32-clang-trunk
>>
>> Also for other architectures, e.g.:
>>
>> http://fate.ffmpeg.org/history.cgi?slot=aarch64-mingw32-clang-trunk
>
>
> OK, thanks for the pointers. I'm not sure whether it would be
> acceptable to require this compilation flag for Windows builds?

I would very much prefer not to require adding -fms-extensions when 
building with Clang - that option unlocks a lot of stuff that we generally 
shouldn't be enabling.

> Can you think of any other ideas?

Right now, mainly doing a #define ff_stat_struct which would require 
updating the calling code. It's not ideal but worse things have been done 
anyway (there's not that many stat calls).

I was exploring the idea of just redefining the struct, but e.g. "typedef 
struct _stati64 win32_stat", but that only works when referring to the 
type as "win32_stat", not "struct win32_stat". So that doesn't seem like a 
good path forward either.

I'd prefer to slow down and think more about other alternatives here, 
rather than rushing forward with adding -fms-extensions.


Also note that currently, we don't even have a proper automatic redirect 
from stat to win32_stat, see the ifdef in libavformat/file.c. So this is 
new development, or, raising the bar even further, in one sense.


// Martin

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Tuesday, May 24, 2022 2:44 PM
> To: Soft Works <softworkz@hotmail.com>
> Cc: Hendrik Leppkes <h.leppkes@gmail.com>; FFmpeg development discussions
> and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> long file names on Windows
> 
> On Tue, 24 May 2022, Soft Works wrote:
> 
> >> -----Original Message-----
> >> From: Martin Storsjö <martin@martin.st>
> >> Sent: Tuesday, May 24, 2022 1:26 PM
> >> To: Soft Works <softworkz@hotmail.com>
> >> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>;
> >> Hendrik Leppkes <h.leppkes@gmail.com>
> >> Subject: RE: [FFmpeg-devel] [PATCH v5 2/2] avformat/os_support: Support
> >> long file names on Windows
> >>
> >> On Tue, 24 May 2022, Soft Works wrote:
> >>
> >>>> but Clang doesn't. (It's possible to use it
> >>>> in Clang too if you enable it with -fms-extensions though.)
> >>>
> >>> Is it possible to compile ffmpeg for Windows using Clang?
> >>> And if yes, does it even work without that flag?
> >>> (assuming it was introduced in order to be able to
> >>> compile Windows stuff).
> >>
> >> Yes, it is possible to build it with Clang without any custom extra
> flags
> >> to enable nondefault modes. In fact, it's tested continuously on FATE
> too:
> >>
> >> http://fate.ffmpeg.org/history.cgi?slot=x86_64-mingw32-clang-trunk
> >>
> >> Also for other architectures, e.g.:
> >>
> >> http://fate.ffmpeg.org/history.cgi?slot=aarch64-mingw32-clang-trunk
> >
> >
> > OK, thanks for the pointers. I'm not sure whether it would be
> > acceptable to require this compilation flag for Windows builds?
> 
> I would very much prefer not to require adding -fms-extensions when
> building with Clang - that option unlocks a lot of stuff that we generally
> shouldn't be enabling.

OK, sure, it always smells when doing something like that just to
achieve a single thing.

> 
> > Can you think of any other ideas?
> 
> Right now, mainly doing a #define ff_stat_struct which would require
> updating the calling code. It's not ideal but worse things have been done
> anyway (there's not that many stat calls).
> 
> I was exploring the idea of just redefining the struct, but e.g. "typedef
> struct _stati64 win32_stat", but that only works when referring to the
> type as "win32_stat", not "struct win32_stat". So that doesn't seem like a
> good path forward either.
> 
> I'd prefer to slow down and think more about other alternatives here,
> rather than rushing forward with adding -fms-extensions.

I have a new idea, see below

> Also note that currently, we don't even have a proper automatic redirect
> from stat to win32_stat, see the ifdef in libavformat/file.c. 

Yes, that can be dropped (once we got it)...


What do you think of the following:

We could define our own win32_stat struct, but not in a way that matches
the Windows API, just matching the POSIX definition (like the consuming 
code expects), e.g.:

struct win_32stat {
    dev_t          st_dev;     /* ID of device containing file */
    ino_t          st_ino;     /* inode number */
    unsigned short st_mode;    /* protection */
    short          st_nlink;   /* number of hard links */
    short          st_uid;     /* user ID of owner */
    short          st_gid;     /* group ID of owner */
    dev_t          st_rdev;    /* device ID (if special file) */
    off_t          st_size;    /* total size, in bytes */
    time_t         st_atime;   /* time of last access */
    time_t         st_mtime;   /* time of last modification */
    time_t         st_ctime;   /* time of last status change */
};

And then, in our win32_stat() function, we call the win api with
the "right" struct and simply copy over the values..:

static int win32_stat(const char *filename_utf8, struct stat *par)
{
    wchar_t *filename_w;
    int ret;
    struct _stati64 winstat;

    if (get_extended_win32_path(filename_utf8, &filename_w))
        return -1;

    if (filename_w) {
        ret = _wstat64(filename_w, &winstat);
        av_free(filename_w);
    } else
        ret = _stat64(filename_utf8, &winstat);

    par->st_dev   = winstat.st_dev;
    par->st_ino   = winstat.st_ino;
    par->st_mode  = winstat.st_mode;
    par->st_nlink = winstat.st_nlink;
    par->st_uid   = winstat.st_uid;
    par->st_gid   = winstat.st_gid;
    par->st_rdev  = winstat.st_rdev;
    par->st_size  = winstat.st_size;
    par->st_atime = winstat.st_atime;
    par->st_mtime = winstat.st_mtime;
    par->st_ctime = winstat.st_ctime;

    return ret;
}

This would be safe and without any weirdness (just a bit more
code).

What do you think about it?


Thanks,
sw