Closed
Bug 1244773
Opened 8 years ago
Closed 8 years ago
Fix ffvpx compilation on mingw.
Categories
(Core :: Audio/Video: Playback, defect, P5)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: jacek, Assigned: jacek)
References
Details
Attachments
(1 file, 2 obsolete files)
1.72 KB,
patch
|
jya
:
review+
|
Details | Diff | Splinter Review |
Building ffvpx on mingw is well supported, but we can't directly use msvc config files. We could add separated ones, but given that it requires just minor changes, I just tweaked existing ones and added its changes to changes.diff.
Attachment #8714349 -
Flags: review?(ajones)
Comment 1•8 years ago
|
||
Comment on attachment 8714349 [details] [diff] [review] fix Review of attachment 8714349 [details] [diff] [review]: ----------------------------------------------------------------- Fly by review as I've done for FreeBSD. These files are auto generated. Having to manually edit them makes future resync difficult. Changes.patch is the patch that was used to generate the ffmpeg source changes. So it doesn't apply here. Maintaining a new set of config files is likely not ideal, so maybe having its own patch/script that from the original self-generated config file generate this config_win*.h; with updated documentation in the README on how to run it. Thank you.
Attachment #8714349 -
Flags: review?(ajones) → review-
Updated•8 years ago
|
Priority: -- → P5
Assignee | ||
Comment 2•8 years ago
|
||
I'm sorry it took so long. The attached version adds new config files that are generated from MSVC config files and instructions in README how to regenerate them.
Attachment #8714349 -
Attachment is obsolete: true
Attachment #8740366 -
Flags: review?(ajones)
Comment on attachment 8740366 [details] [diff] [review] fix v2 Review of attachment 8740366 [details] [diff] [review]: ----------------------------------------------------------------- Jean-Yves needs to review this.
Attachment #8740366 -
Flags: review?(ajones) → review?(jyavenard)
Comment 4•8 years ago
|
||
Comment on attachment 8740366 [details] [diff] [review] fix v2 Review of attachment 8740366 [details] [diff] [review]: ----------------------------------------------------------------- I don't think we want yet another config file that no one will be able to update when needed because it's not really a supported config Please take the same approach as I described in https://bugzilla.mozilla.org/show_bug.cgi?id=1239550#c13, that is have your stuff in config.h and add at the end something like: #if defined(XP_WIN) && !defined(_MSC_VER) #if !defined(HAVE_64BIT_BUILD) #undef HAVE_MM_EMPTY #define HAVE_MM_EMPTY 0 #endif #undef HAVE_LIBC_MSVCRT #define HAVE_LIBC_MSVCRT 0 #endif as really, that's all you need. much simpler and easier to maintain.
Attachment #8740366 -
Flags: review?(jyavenard) → review-
Assignee | ||
Comment 5•8 years ago
|
||
Thank you for the review. I attached a fix that you described, it's indeed much cleaner.
Attachment #8740366 -
Attachment is obsolete: true
Attachment #8744583 -
Flags: review?(jyavenard)
Updated•8 years ago
|
Attachment #8744583 -
Flags: review?(jyavenard) → review+
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5680a55b2ec1
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•