-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial Windows support #38
Conversation
@sindresorhus |
README.md
Outdated
@@ -11,13 +11,16 @@ | |||
$ npm install aperture | |||
``` | |||
|
|||
*Requires macOS 10.10 or later.* | |||
requires: *macOS: 10.10 or later.* | *Windows: ffmpeg* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be:
*Requires minimum macOS 10.10 or Windows 7.*
README.md
Outdated
|
||
|
||
## Usage | ||
|
||
```js | ||
const aperture = require('aperture')(); | ||
const aperture = require('aperture')({ | ||
// ffmpegBinary is *required* on Windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is moot. It's already explained in the docs section.
README.md
Outdated
##### ffmpegBinary | ||
|
||
Type: `string`<br> | ||
**Required on windows** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
windows => Windows
index.js
Outdated
ffmpegArgs.push( | ||
'-video_size', `${cropArea.width}x${cropArea.height}`, | ||
'-f', 'gdigrab', | ||
'-i', 'desktop', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above two arguments can be in the initial array as they appear the same in both conditions.
@@ -1,6 +1,6 @@ | |||
<p> | |||
<h1 align="center">Aperture</h1> | |||
<h4 align="center">Record the screen on macOS</h4> | |||
<h4 align="center">Record the screen on macOS & Windows</h4> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update the package.json description too.
Since |
Is there any interest in finishing this? I don't use Windows and never will use this lib on windows, but if there are people that want this I could pick it up and finish it. Thoughts? |
Not really. I only care about the macOS part. I just reviewed this to be nice. |
For now, I don't think we can maintain this without a contributor that has windows. I'll be building a PC at some point this year and then I can maintain it, but for now I don't think we can support it properly. 😢 EDIT: There are people who want this (see our most popular Kap issue) but no one interested in maintaining it. It has to go hand-in-hand. |
I just did this to be nice too 😄 But as I use macOS as my main OS, I can't really maintain it. Should I let this PR be open, and wait for someone to come along (windows maintainer) or close it? Edit: do you want to find a windows maintainer for this? Should we look over in the kap issue maybe if someone is willing to take this? |
I'll close this. @albinekb Can you open an issue instead here about adding Windows support? And link to this PR as a starting point. |
I use Windows as my main OS besides Arch Linux and could try to pick this up if you guys want me to. (And extend it further with Linux later) |
At this point we won't be expanding the scope of Kap, however, if there was a high quality unofficial fork with support for another OS I'd argue that we should link to that. The reasoning being that maintaining this and any comparable open-source project is a lot of work, and the interest for most stops at the idea of doing it. Additionally, the insight we have tells us that Windows and Linux support will require more work with less people benefitting from it. |
|
It's always great to hear from people that would like Kap on Windows. We'd love to see someone fork Kap and port it to Windows @levrik! To show your interest your can leave a 👍 on #46 and wulkano/Kap#456. |
Thanks for your comment @shawwn, but please refrain from posting "+1" and use 👍 on the related issues instead. When you do that we can sort issues based on reactions, making your voice heard and helping us prioritise. Remember that we work on a lot of projects, some of us in the tens and hundreds, and every single +1 is a notification/activity log item that just causes noise.
|
#37 without linux support
todo
http://zornsoftware.codenature.info/blog/record-screen-audio-with-ffmpeg.html