Skip to content
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

Adding Black Magic Design Decklink support to FFMPEG Preset #543

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

masterav10
Copy link

This pull request enables decklink device support if the appropriate Black Magic SDK is installed on the build machine.

Currently enabled for only Windows x86_64. The SDK supports windows, linux, and mac, so we can probably add decklink to those platforms as well.

I have verified that the devices are enumerated through libavdevice on a machine with a Black Magic Decklink Quad. Haven't had a chance to hook up to the them, though.

In addition to this pull request, we need to do the following:

  1. Update msys config to include the following:
    pacman -S mingw-w64-{i686,x86_64}-tools-git dos2unix
  2. Install Black Magic Desktop SDK on the appveyor server.

If we don't want the backoff approach as taken here, we could include the headers directly into the repository under src/main/resources. Then we don't have to the above. However, it places more burden on the maintainer, since you have to download the SDK and run midl to get the appropriate headers. Let me know if you'd rather change to this approach.

@saudet
Copy link
Member

saudet commented Apr 1, 2018

This links against a dynamic library we can't bundle, am I correct?

@masterav10
Copy link
Author

Shouldn't be. Which library are you referring to? The only thing it should be linked to are native libraries. Here is the one I built yesterday before the pull request.

image

@saudet
Copy link
Member

saudet commented Apr 1, 2018

Ok, looks good. Please add the installation of the SDK in the install-windows.sh script, and do the same for Linux and Mac, if possible. Thanks!

@masterav10
Copy link
Author

masterav10 commented Apr 1, 2018

Yeah, I'm going to have to make some changes to make this work. There is no way to download the SDK automatically that I can see (used to be available). And even if we could, their license for the SDK as a whole is different than the license for the individual files (which is more permissive).

Next best option is to simply copy the appropriate header files directly into the repo. When a new update occurs, simply download the new SDK and install the appropriate headers again. Annoying, but doable.

Zeronae used to support it but removed said support, claiming incompatible license.

I cannot find any section in either of the license agreements that would cause a conflict with an existing license we have, and as you have seen, we don't like to any binaries. So I'm going to proceed forth with the intent of adding decklink support. I'll ping when I'm done.

@saudet
Copy link
Member

saudet commented Apr 4, 2018

If FFmpeg just needs a few small header files, we can just add them to this repository in src/main/resources/ and use them from there.

@masterav10
Copy link
Author

The following platforms should now support decklink via FFMPEG:
macosx, linux-x86, linux-x86_64, windows-x86, windows-x86_64.

Verified the Windows build with my local machine. Will test against an actual decklink in the coming week.

@saudet
Copy link
Member

saudet commented Apr 5, 2018

Wow, that's a lot of header files, let's put that somewhere else...

@masterav10
Copy link
Author

No problem. Will make a separate repo here on github. Gonna be a couple of days before I get to it, potentially.

@saudet
Copy link
Member

saudet commented Apr 6, 2018

It looks like we can download from their site without registering though. Let's do that?

@masterav10
Copy link
Author

That's for the drivers. We need the SDK. Or did you find a different link?

@saudet
Copy link
Member

saudet commented Apr 7, 2018

I see. These files already appear to be available in other repositories though, for example:
https://github.com/GStreamer/gst-plugins-bad/tree/master/sys/decklink/linux
We could just pull from there maybe? Although the license does appear to be a concern:
https://patchwork.ffmpeg.org/patch/3230/
We might want to contact DeckLink directly to sort this out...

@masterav10
Copy link
Author

Looking into this.

I can now detect when a source is not available using the flags field of the AVPacket.
@masterav10
Copy link
Author

Finally got an answer back. The EULA for the SDK is designed so that a single public point is available for downloading the SDK. This does in fact prevent us from using their headers, since the GPL requires that we provide all sources (including the decklink headers) to anyone we distribute our libraries to.

If possible, I would like to revert back to my previous implementation which will detect the SDK on the machine where the build is happening. Since we won't have the SDK installed on the CI servers, it won't add decklink to your normal distribution. Then I can make local builds of FFmpeg with decklink support without the need to maintain a separate branch.

Seem reasonable?

@saudet
Copy link
Member

saudet commented May 1, 2018 via email

masterav10 added 4 commits May 5, 2018 11:56
Added a patch to remove references to unredistributable files from within the FFmpeg source code.
Added a patch to remove references to unredistributable files from within the FFmpeg source code.
@saudet
Copy link
Member

saudet commented May 22, 2018

For now though, please remove from this PR the files from the SDK so we can merge this! Thanks

@masterav10
Copy link
Author

I should be able to finish this by the weekend.

@masterav10
Copy link
Author

This adds support for windows 32 and 64. I've only tested the 64-bit version, but it works.

Added some patches to ignore SDK files; this keeps our license kosher. There is also a patch to the FFmpeg decklink code so that I can tell if a video source stops producing frames. Let me know if you need anything else.

@saudet
Copy link
Member

saudet commented Jun 7, 2018

Looking good, but DeckLinkAPI.h and DeckLinkAPI_i.c are still there, could you remove them? Let's also remove LICENSE.DeckLinkAPI since it is misleading.

Could we also consolidate all the patches into one big decklink.patch? Thanks!

@saudet
Copy link
Member

saudet commented Jun 7, 2018

Are you saying that DeckLinkAPI.h and DeckLinkAPI_i.c are not part of the SDK? Sounds like they were generated from the SDK... And what's with the DeckLinkAPI.LICENSE file?

@masterav10
Copy link
Author

They are indeed generated from the SDK. I believe that allows them to fall under the permissive license (which I included). Would you prefer them elsewhere? I guess we could generate them, but you'd have to install the SDK on the server, along with some additional msys libs.

I will take care of the patches.

@saudet
Copy link
Member

saudet commented Jun 7, 2018

Right, so maybe it's alright. Is this what your contact at DeckLink said?

@masterav10
Copy link
Author

That is correct; the files in the SDK cannot be distributed, but files generated from the SDK can be distributed under the permissive license.

@saudet
Copy link
Member

saudet commented Jun 8, 2018

Is this in the license that came with the SDK you have?

Developer agrees not to distribute the Materials or any derivative works created therewith without the express written permission of an authorized Blackmagic Design officer or employee.

https://notabug.org/RiCON/decklink-headers/raw/d0ead620884ab95954f55646f1acddb100b7ebdf/SDK%20License.txt

It sounds like we would still need their permission. Can you forward me the message you got from DeckLink about this?

@masterav10
Copy link
Author

masterav10 commented Jun 8, 2018

That is not the correct license. You can read the full license by attempting to download the SDK (copy it into notepad). Derivative works are not mentioned.

Permitted Use:

...making the Licensee's existing software compatible with the Licensor's products

@masterav10
Copy link
Author

Builds look like they have the appropriate stuff. Do you want to wait for BMD to look at this, or just go for it? Based on their comments, as long as we include the license like we have, we should be good.

@saudet
Copy link
Member

saudet commented Jun 15, 2018

@masterav10 Yeah, if we could have them look at this PR just to make sure that would be great. Otherwise looks good, though does this not build on Linux or Mac? We'd need something more for that?

@saudet
Copy link
Member

saudet commented Jul 1, 2018

Any news from them? Have the OKed this PR?

@masterav10
Copy link
Author

No word yet. I could try to send another email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants