1237471 - New ffvpx code redefines vsnprintf and snprintf on windows unconditionally conflicting with VS2015

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Bas Schouten (:bas.schouten)]

ffvpxcommon.mozbuild defines snprintf and vsnprintf. VS2015 actually has these functions as part of the standard library and it does not like us redefining them.

Comment 1

[Jean-Yves Avenard [:jya]]

thanks for reporting that. will take over.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1237471: Fix VS2015 build by not using custom snprintf/vsnprintf functions when having VS2015's. r=jya

Review commit: https://reviewboard.mozilla.org/r/29799/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29799/

Comment 3

[Mike Hommey [:glandium]]

https://reviewboard.mozilla.org/r/29799/#review26641

::: media/ffvpx/libavutil/avutil-54-oldmsvc.symbols:1
(Diff revision 1)
> +av_add_q

You could #include avutil-54.symbols instead of putting everything in here.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Fix VS2015 compilation. rpending=glandium

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8704944 [details] [diff] [review]
Fix VS2015 compilation. rpending=glandium

Mike, I see that you're around.. If you could have a look quickly.

Verified fixed locally.

I'll push it shortly if you're not there...

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8704926 [details]
MozReview Request: Bug 1237471: Fix VS2015 build by not using custom snprintf/vsnprintf functions when having VS2015's. r=jya

I'd prefer not to have a second symbols file, and that will break all non-2015 build.

should have been:
if CONFIG['_MSC_VER'] && CONFIG['_MSC_VER'] < '1900'

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e597b8795fbe

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e597b8795fbe

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8704944 [details] [diff] [review]
Fix VS2015 compilation. rpending=glandium

Review of attachment 8704944 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/ffvpx/libavutil/dummy_funcs.c
@@ +20,5 @@
> +
> +// VS2015 (and later) now provides snprintf. As we have a global symbols
> +// file that do not allow conditional export, we create a dummy entry instead.
> +#if defined(_MSC_VER) && _MSC_VER >= 1900
> +int avpriv_snprintf(char *s, size_t n, const char *fmt, ...) { return 0; }

I hope this is not used...

Comment 10

[Jean-Yves Avenard [:jya]]

(In reply to Mike Hommey [:glandium] from comment #9)
> Comment on attachment 8704944 [details] [diff] [review]
> Fix VS2015 compilation. rpending=glandium
> 
> Review of attachment 8704944 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/ffvpx/libavutil/dummy_funcs.c
> @@ +20,5 @@
> > +
> > +// VS2015 (and later) now provides snprintf. As we have a global symbols
> > +// file that do not allow conditional export, we create a dummy entry instead.
> > +#if defined(_MSC_VER) && _MSC_VER >= 1900
> > +int avpriv_snprintf(char *s, size_t n, const char *fmt, ...) { return 0; }
> 
> I hope this is not used...

It isn't... But as the .symbols file list it, we have to make a dummy one (better alternative would be to have conditional support in .symbols file, so the symbol could be within #if _MSC_VER < 1900 .. #endif)