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

RESOLVED FIXED in Firefox 46

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: bas.schouten, Assigned: jya)

Tracking

unspecified
mozilla46
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

4 years ago
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.
Assignee

Comment 1

4 years ago
thanks for reporting that. will take over.
Assignee: bas → jyavenard
Assignee

Updated

4 years ago
Component: Audio/Video → Build Config
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.
Assignee

Comment 5

4 years ago
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...
Flags: needinfo?(mh+mozilla)
Assignee

Comment 6

4 years ago
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'
Attachment #8704926 - Flags: review?(jyavenard)

Comment 8

4 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e597b8795fbe
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
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...
Flags: needinfo?(mh+mozilla)
Assignee

Comment 10

4 years ago
(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)
Depends on: 1277155

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.