[FFmpeg-devel] [PATCH] Fix MSVC warnings about possible value truncation.

Peter Kasting pkasting at google.com
Sat Aug 30 01:38:28 CEST 2014


On Fri, Aug 29, 2014 at 4:26 PM, Reimar Döffinger <Reimar.Doeffinger at gmx.de>
wrote:

> First, this needs very, very careful review. I am not at all convinced
> that these will not change behaviour.
>

I strongly agree that it needs careful review.

I think that reinforces why a change like this is important.  These sorts
of value truncations shouldn't just be invisibly, implicitly happening,
since in many cases they can have important consequences.  If your code
stores a double in a float, you should be thinking about whether precision
loss is important.  If it stores a floating-point value in an integral one,
you should be wondering if you need to round rather than truncate.  If it
stores a 64-bit int in a 32-bit one, you should be thinking about potential
integer overflow and whether the storage type needs to be increased to e.g.
handle large files or long data streams.  Etc.

Second, I believe powf and sinf are less commonly available than pow and
> sin, so I think this will break compilation on some platforms (but haven't
> double-checked).
>

These are part of C99, so if platforms are at least C99-compliant, they
should work.  Does FFMPEG support pre-C99 targets?

(After I wrote this, James Almer noted this probably isn't an issue.)

Third, _if_ we want this, I would expect we want to enable the
> corresponding gcc warning as well.
>

That seems like an excellent idea to me.  I don't build with gcc, though,
so I'm not sure which warning(s) correspond.

Only a minority of developers has access to MSVC and I consider this kind
> of thing not maintainable if most people will not get the warnings.
>

That's fair.  Chromium is building FFMPEG with MSVC (and staying in sync
with upstream pretty closely), so we would fix regressions that crept in,
but having these sorts of cases caught by gcc would obviously be preferable.

PK


More information about the ffmpeg-devel mailing list