diff mbox

[FFmpeg-devel] On in-tree external headers

Message ID 1e136607-9a69-de80-1733-93b1070eeb15@jkqxz.net
State Superseded
Headers show

Commit Message

Mark Thompson Nov. 5, 2017, 2:24 p.m. UTC
On 30/10/17 19:51, Mark Thompson wrote:
> 
> Hi all,
> 
> The recent submission of AMD AMF patches including a builtin header prompted me to think further about what external API headers should actually be included in the tree.
> 
> For the AMD headers (like V4L2 previously), it seems entirely silly to include them - they are available upstream in a free form which can easily be packaged and available on any build machine to use.
> 
> However, there is the problem that this in some sense places them second-class to the Nvidia implementation, which includes all headers in-tree and automatically enables itself - any normal build for x86 will include Nvidia support by default if the user doesn't explicitly disable it.  The effect of that is essentially that the ffmpeg project is facilitating Nvidia's anti-open behaviour by including the headers, which is I think something we really shouldn't be doing.
> 
> So: can we please precisely codify under what circumstances external headers should be included in the ffmpeg tree?
> 
> As an initial position for consideration, I propose "no external headers may be included in the ffmpeg tree".  That is, the contents of the compat/ directory should only be OS/compiler compatibility workarounds, not any functional headers.
> 
> Would anyone like to propose an alternative position?  (Precisely defined - "what we have now" would need some clarification of what that actually means so that we can apply it consistently to future requests.)
> 
> I also ask that, whatever discussion here ends up with, the voting committee should vote to ratify it so that we don't have to discuss it again every time someone proposes including headers.

No alternatives were proposed, so I would like to call a vote on adopting the following clear policy:

"No external headers may be included in the ffmpeg tree."

(A corresponding documentation change is below, which tries to cover the related packaging point.)

Does anybody want to offer any other options to vote on?  I suggest starting the vote tomorrow, to run for one week.

Thanks,

- Mark

Comments

Carl Eugen Hoyos Nov. 5, 2017, 6:28 p.m. UTC | #1
2017-11-05 15:24 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
> On 30/10/17 19:51, Mark Thompson wrote:

> "No external headers may be included in the ffmpeg tree."

So you suggest to remove the Nvidia header?

Carl Eugen
Mark Thompson Nov. 5, 2017, 6:35 p.m. UTC | #2
On 05/11/17 18:28, Carl Eugen Hoyos wrote:
> 2017-11-05 15:24 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
>> On 30/10/17 19:51, Mark Thompson wrote:
> 
>> "No external headers may be included in the ffmpeg tree."
> 
> So you suggest to remove the Nvidia header?
If that specific policy is adopted then it would have to be.

Alternative proposals are welcome.

Thanks,

- Mark
Carl Eugen Hoyos Nov. 5, 2017, 6:42 p.m. UTC | #3
2017-11-05 19:35 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
> On 05/11/17 18:28, Carl Eugen Hoyos wrote:
>> 2017-11-05 15:24 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
>>> On 30/10/17 19:51, Mark Thompson wrote:
>>
>>> "No external headers may be included in the ffmpeg tree."
>>
>> So you suggest to remove the Nvidia header?
> If that specific policy is adopted then it would have to be.

Then I don't think this policy is useful.

Iirc, there is a second external header, I have never used
either of those but judging from user response, both features
are heavily used making a removal a no-go.

> Alternative proposals are welcome.

I don't really understand:
It was mentioned several times in the relevant thread - and
I completely agree - that the fact that one external header
is present is no argument to add another. (And iirc - and
again I agree - it was also argued that pushing on this
argument is not a good idea.)
If you believe adding the AMD header makes sense, it may
possibly be added, if you are against it, it will likely not be
added.

Carl Eugen
James Almer Nov. 5, 2017, 7:07 p.m. UTC | #4
On 11/5/2017 3:42 PM, Carl Eugen Hoyos wrote:
> 2017-11-05 19:35 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
>> On 05/11/17 18:28, Carl Eugen Hoyos wrote:
>>> 2017-11-05 15:24 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
>>>> On 30/10/17 19:51, Mark Thompson wrote:
>>>
>>>> "No external headers may be included in the ffmpeg tree."
>>>
>>> So you suggest to remove the Nvidia header?
>> If that specific policy is adopted then it would have to be.
> 
> Then I don't think this policy is useful.
> 
> Iirc, there is a second external header, I have never used
> either of those but judging from user response, both features
> are heavily used making a removal a no-go.

Removal of the nvidia headers from the tree could be done by creating a
separate repository specifically for them. The work done to get them in
this form is not something that should be lost.

> 
>> Alternative proposals are welcome.
> 
> I don't really understand:
> It was mentioned several times in the relevant thread - and
> I completely agree - that the fact that one external header
> is present is no argument to add another. (And iirc - and
> again I agree - it was also argued that pushing on this
> argument is not a good idea.)
> If you believe adding the AMD header makes sense, it may
> possibly be added, if you are against it, it will likely not be
> added.
> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/doc/developer.texi b/doc/developer.texi
index a7b4f1d737..3aeb727729 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -381,6 +381,15 @@  Never write to unallocated memory, never write over the end of arrays,
 always check values read from some untrusted source before using them
 as array index or other risky things.
 
+@subheading External headers
+Do not include external headers directly in FFmpeg.  Most packages provide
+suitable headers and linkable libraries when installed, and these should be
+used.  References to external packages in configure should use pkg-config
+if possible to find paths and options - ad-hoc build tests with hard-coded
+paths are to be avoided, but can be used if there is no other possibility.
+If an external package does not provide usable headers or libraries then
+please add directions for how to use it to the documentation.
+
 @subsection Documentation/Other
 @subheading Subscribe to the ffmpeg-cvslog mailing list.
 It is important to do this as the diffs of all commits are sent there and