[FFmpeg-devel] [PATCH] lavu: add snprintf(), vsnprint() and strtod() replacements for MS runtime.

[Ronald S. Bultje]

Hi,

On Mon, Aug 27, 2012 at 8:59 PM, jamal <jamrial at gmail.com> wrote:
> On 27/08/12 11:11 PM, Ronald S. Bultje wrote:
>>>
>>> I think configure is including strtod.h to CFLAGS even though it shouldn't.
>>> Check config.mak, -includecompat/strtod.h is at the end of CFLAGS even though HAVE_BROKEN_STRTOD is false.
>>> It seems to do it even when compiling for linux.
>>>
>>> Changing "disabled broken_strtod || force_include compat/strtod.h" to "enabled broken_strtod && force_include compat/strtod.h" in configure fixed this for me.
>>
>> Uhm... Admittedly, I didn't test this on Linux, but uh ... Isn't that
>> the same thing? I don't think I understand shell scripting at all.
>>
>> Anyway, you can change as suggested or I can send a new patch, should
>> be OK either way.
>>
>> Ronald
>>
> For some reason, even though broken_strtod isn't enabled, it isn't explicitly disabled either, or so to say. So "disabled broken_strtod" seems to always return false even if broken_strtod was never enabled.
>
> enabled() only returns true when the variable in question is explicitly set to enabled, and false otherwise. This is the expected behavior.
> Same way disabled() only returns true when the variable in question is explicitly set to disabled, and false otherwise. Judging by what happened with your patch, i imagine this is not the expected behavior.
> So yeah, both lines above are in essence the same thing, but the disabled() function isn't working as it should.
>
> That being said, i really think that this would be much better as part of a new os_support header and C files on libavutil (Since Sabatini mentioned this doesn't belong in the ones from libavformat). It would get rid of the need to append an include flag to CFLAGS, and would also prevent Make from recompiling every single file when the corresponding header gets modified, recompiling only those that explicitly need it and include it instead.
> Or maybe including the corresponding files from the compat folder on the needed files just like how it's currently being done with compat/getopt.c
> It seems a bit hackish to me like this.

Well, that's intentional. See, these functions are an essential part
of C99, and we expect them to be there. Our files include the proper
headers and expect it to work. On a broken clib like MSVCRT, this
isn't the case. So we can either clutter our files with headers that
shouldn't be there, or we can hack the build system a little and keep
our sources clean. I chose the second. That's also why the files are
in compat/, not in libavutil/compat/ or so.

As for getopt, I'm the person that put it there, and I think in
hindsight that was a poor idea. I'll probably fix that later to work
like this, so that our sources don't have to be aware that there's
clibs out there that are screwed up. I think it's somewhat cleaner.

Can that be used for libavformat? Yes, maybe. I hope someone else but
me will volunteer to fix that, or maybe we just keep it as-is, I mean,
it works, why change it, right? The #include "os_support.h" all over
the place in lavf isn't pretty, but then again, it's already there
anyway...

Ronald