|
|
<!DOCTYPE html> |
|
|
<html> |
|
|
|
|
|
<head> |
|
|
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"> |
|
|
<title>Developer Documentation</title> |
|
|
|
|
|
<meta name="description" content="Developer Documentation"> |
|
|
<meta name="keywords" content="Developer Documentation"> |
|
|
<meta name="resource-type" content="document"> |
|
|
<meta name="distribution" content="global"> |
|
|
<meta name="Generator" content="makeinfo"> |
|
|
<meta name="viewport" content="width=device-width,initial-scale=1"> |
|
|
|
|
|
<link href="#SEC_Contents" rel="contents" title="Table of Contents"> |
|
|
<style type="text/css"> |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
</style> |
|
|
|
|
|
|
|
|
</head> |
|
|
|
|
|
<body lang="en"> |
|
|
|
|
|
|
|
|
<div class="top-level-extent" id="SEC_Top"> |
|
|
|
|
|
|
|
|
<div class="element-contents" id="SEC_Contents"> |
|
|
<h2 class="contents-heading">Table of Contents</h2> |
|
|
|
|
|
<div class="contents"> |
|
|
|
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Introduction" href="#Introduction">1 Introduction</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Contributing-code" href="#Contributing-code">1.1 Contributing code</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Coding-Rules-1" href="#Coding-Rules-1">2 Coding Rules</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Language" href="#Language">2.1 Language</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-SIMD_002fDSP-1" href="#SIMD_002fDSP-1">2.1.1 SIMD/DSP</a></li> |
|
|
<li><a id="toc-Other-languages" href="#Other-languages">2.1.2 Other languages</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Code-formatting-conventions" href="#Code-formatting-conventions">2.2 Code formatting conventions</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Vim-configuration" href="#Vim-configuration">2.2.1 Vim configuration</a></li> |
|
|
<li><a id="toc-Emacs-configuration" href="#Emacs-configuration">2.2.2 Emacs configuration</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Comments" href="#Comments">2.3 Comments</a></li> |
|
|
<li><a id="toc-Naming-conventions-1" href="#Naming-conventions-1">2.4 Naming conventions</a></li> |
|
|
<li><a id="toc-Miscellaneous-conventions" href="#Miscellaneous-conventions">2.5 Miscellaneous conventions</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Development-Policy-1" href="#Development-Policy-1">3 Development Policy</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Patches_002fCommitting" href="#Patches_002fCommitting">3.1 Patches/Committing</a></li> |
|
|
<li><a id="toc-Code" href="#Code">3.2 Code</a></li> |
|
|
<li><a id="toc-Library-public-interfaces" href="#Library-public-interfaces">3.3 Library public interfaces</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Adding-new-interfaces" href="#Adding-new-interfaces">3.3.1 Adding new interfaces</a></li> |
|
|
<li><a id="toc-Removing-interfaces-1" href="#Removing-interfaces-1">3.3.2 Removing interfaces</a></li> |
|
|
<li><a id="toc-Major-version-bumps-1" href="#Major-version-bumps-1">3.3.3 Major version bumps</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Documentation_002fOther" href="#Documentation_002fOther">3.4 Documentation/Other</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Submitting-patches-1" href="#Submitting-patches-1">4 Submitting patches</a></li> |
|
|
<li><a id="toc-New-codecs-or-formats-checklist" href="#New-codecs-or-formats-checklist">5 New codecs or formats checklist</a></li> |
|
|
<li><a id="toc-Patch-submission-checklist" href="#Patch-submission-checklist">6 Patch submission checklist</a></li> |
|
|
<li><a id="toc-Patch-review-process" href="#Patch-review-process">7 Patch review process</a></li> |
|
|
<li><a id="toc-Regression-tests-1" href="#Regression-tests-1">8 Regression tests</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Adding-files-to-the-fate_002dsuite-dataset" href="#Adding-files-to-the-fate_002dsuite-dataset">8.1 Adding files to the fate-suite dataset</a></li> |
|
|
<li><a id="toc-Visualizing-Test-Coverage" href="#Visualizing-Test-Coverage">8.2 Visualizing Test Coverage</a></li> |
|
|
<li><a id="toc-Using-Valgrind" href="#Using-Valgrind">8.3 Using Valgrind</a></li> |
|
|
</ul></li> |
|
|
<li><a id="toc-Release-process-1" href="#Release-process-1">9 Release process</a> |
|
|
<ul class="toc-numbered-mark"> |
|
|
<li><a id="toc-Criteria-for-Point-Releases-1" href="#Criteria-for-Point-Releases-1">9.1 Criteria for Point Releases</a></li> |
|
|
<li><a id="toc-Release-Checklist" href="#Release-Checklist">9.2 Release Checklist</a></li> |
|
|
</ul></li> |
|
|
</ul> |
|
|
</div> |
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Introduction"> |
|
|
<h2 class="chapter">1 Introduction</h2> |
|
|
|
|
|
<p>This text is concerned with the development <em class="emph">of</em> FFmpeg itself. Information |
|
|
on using the FFmpeg libraries in other programs can be found elsewhere, e.g. in: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>the installed header files |
|
|
</li><li><a class="url" href="http://ffmpeg.org/doxygen/trunk/index.html">the Doxygen documentation</a> |
|
|
generated from the headers |
|
|
</li><li>the examples under <samp class="file">doc/examples</samp> |
|
|
</li></ul> |
|
|
|
|
|
<p>If you modify FFmpeg code for your own use case, you are highly encouraged to |
|
|
<em class="emph">submit your changes back to us</em>, using this document as a guide. There are |
|
|
both pragmatic and ideological reasons to do so: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>Maintaining external changes to keep up with upstream development is |
|
|
time-consuming and error-prone. With your code in the main tree, it will be |
|
|
maintained by FFmpeg developers. |
|
|
</li><li>FFmpeg developers include leading experts in the field who can find bugs or |
|
|
design flaws in your code. |
|
|
</li><li>By supporting the project you find useful you ensure it continues to be |
|
|
maintained and developed. |
|
|
</li></ul> |
|
|
|
|
|
<p>For more detailed legal information about the use of FFmpeg in |
|
|
external programs read the <samp class="file">LICENSE</samp> file in the source tree and |
|
|
consult <a class="url" href="https://ffmpeg.org/legal.html">https://ffmpeg.org/legal.html</a>. |
|
|
</p> |
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#Contributing-code" accesskey="1">Contributing code</a></li> |
|
|
</ul> |
|
|
<div class="section-level-extent" id="Contributing-code"> |
|
|
<h3 class="section">1.1 Contributing code</h3> |
|
|
|
|
|
<p>All proposed code changes should be submitted for review to |
|
|
<a class="url" href="mailto:ffmpeg-devel@ffmpeg.org">the development mailing list</a>, as |
|
|
described in more detail in the <a class="ref" href="#Submitting-patches">Submitting patches</a> chapter. The code |
|
|
should comply with the <a class="ref" href="#Development-Policy">Development Policy</a> and follow the <a class="ref" href="#Coding-Rules">Coding Rules</a>. |
|
|
The developer making the commit and the author are responsible for their changes |
|
|
and should try to fix issues their commit causes. |
|
|
</p> |
|
|
<a class="anchor" id="Coding-Rules"></a></div> |
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Coding-Rules-1"> |
|
|
<h2 class="chapter">2 Coding Rules</h2> |
|
|
|
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#Language" accesskey="1">Language</a></li> |
|
|
<li><a href="#Code-formatting-conventions" accesskey="2">Code formatting conventions</a></li> |
|
|
<li><a href="#Comments" accesskey="3">Comments</a></li> |
|
|
<li><a href="#Naming-conventions-1" accesskey="4">Naming conventions</a></li> |
|
|
<li><a href="#Miscellaneous-conventions" accesskey="5">Miscellaneous conventions</a></li> |
|
|
</ul> |
|
|
<div class="section-level-extent" id="Language"> |
|
|
<h3 class="section">2.1 Language</h3> |
|
|
|
|
|
<p>FFmpeg is mainly programmed in the ISO C99 language, extended with: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>Atomic operations from C11 <samp class="file">stdatomic.h</samp>. They are emulated on |
|
|
architectures/compilers that do not support them, so all FFmpeg-internal code |
|
|
may use atomics without any extra checks. However, <samp class="file">stdatomic.h</samp> must not |
|
|
be included in public headers, so they stay C99-compatible. |
|
|
</li></ul> |
|
|
|
|
|
<p>Compiler-specific extensions may be used with good reason, but must not be |
|
|
depended on, i.e. the code must still compile and work with compilers lacking |
|
|
the extension. |
|
|
</p> |
|
|
<p>The following C99 features must not be used anywhere in the codebase: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>variable-length arrays; |
|
|
|
|
|
</li><li>complex numbers; |
|
|
|
|
|
</li><li>mixed statements and declarations. |
|
|
</li></ul> |
|
|
|
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#SIMD_002fDSP-1" accesskey="1">SIMD/DSP</a></li> |
|
|
<li><a href="#Other-languages" accesskey="2">Other languages</a></li> |
|
|
</ul> |
|
|
<div class="subsection-level-extent" id="SIMD_002fDSP-1"> |
|
|
<h4 class="subsection">2.1.1 SIMD/DSP</h4> |
|
|
<a class="anchor" id="SIMD_002fDSP"></a> |
|
|
<p>As modern compilers are unable to generate efficient SIMD or other |
|
|
performance-critical DSP code from plain C, handwritten assembly is used. |
|
|
Usually such code is isolated in a separate function. Then the standard approach |
|
|
is writing multiple versions of this function – a plain C one that works |
|
|
everywhere and may also be useful for debugging, and potentially multiple |
|
|
architecture-specific optimized implementations. Initialization code then |
|
|
chooses the best available version at runtime and loads it into a function |
|
|
pointer; the function in question is then always called through this pointer. |
|
|
</p> |
|
|
<p>The specific syntax used for writing assembly is: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>NASM on x86; |
|
|
|
|
|
</li><li>GAS on ARM. |
|
|
</li></ul> |
|
|
|
|
|
<p>A unit testing framework for assembly called <code class="code">checkasm</code> lives under |
|
|
<samp class="file">tests/checkasm</samp>. All new assembly should come with <code class="code">checkasm</code> tests; |
|
|
adding tests for existing assembly that lacks them is also strongly encouraged. |
|
|
</p> |
|
|
</div> |
|
|
<div class="subsection-level-extent" id="Other-languages"> |
|
|
<h4 class="subsection">2.1.2 Other languages</h4> |
|
|
|
|
|
<p>Other languages than C may be used in special cases: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>Compiler intrinsics or inline assembly when the code in question cannot be |
|
|
written in the standard way described in the <a class="ref" href="#SIMD_002fDSP">SIMD/DSP</a> section. This |
|
|
typically applies to code that needs to be inlined. |
|
|
|
|
|
</li><li>Objective-C where required for interacting with macOS-specific interfaces. |
|
|
</li></ul> |
|
|
|
|
|
</div> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Code-formatting-conventions"> |
|
|
<h3 class="section">2.2 Code formatting conventions</h3> |
|
|
|
|
|
<p>There are the following guidelines regarding the indentation in files: |
|
|
</p> |
|
|
<ul class="itemize mark-bullet"> |
|
|
<li>Indent size is 4. |
|
|
|
|
|
</li><li>The TAB character is forbidden outside of Makefiles as is any |
|
|
form of trailing whitespace. Commits containing either will be |
|
|
rejected by the git repository. |
|
|
|
|
|
</li><li>You should try to limit your code lines to 80 characters; however, do so if |
|
|
and only if this improves readability. |
|
|
|
|
|
</li><li>K&R coding style is used. |
|
|
</li></ul> |
|
|
<p>The presentation is one inspired by ’indent -i4 -kr -nut’. |
|
|
</p> |
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#Vim-configuration" accesskey="1">Vim configuration</a></li> |
|
|
<li><a href="#Emacs-configuration" accesskey="2">Emacs configuration</a></li> |
|
|
</ul> |
|
|
<div class="subsection-level-extent" id="Vim-configuration"> |
|
|
<h4 class="subsection">2.2.1 Vim configuration</h4> |
|
|
<p>In order to configure Vim to follow FFmpeg formatting conventions, paste |
|
|
the following snippet into your <samp class="file">.vimrc</samp>: |
|
|
</p><div class="example"> |
|
|
<pre class="example-preformatted">" indentation rules for FFmpeg: 4 spaces, no tabs |
|
|
set expandtab |
|
|
set shiftwidth=4 |
|
|
set softtabstop=4 |
|
|
set cindent |
|
|
set cinoptions=(0 |
|
|
" Allow tabs in Makefiles. |
|
|
autocmd FileType make,automake set noexpandtab shiftwidth=8 softtabstop=8 |
|
|
" Trailing whitespace and tabs are forbidden, so highlight them. |
|
|
highlight ForbiddenWhitespace ctermbg=red guibg=red |
|
|
match ForbiddenWhitespace /\s\+$\|\t/ |
|
|
" Do not highlight spaces at the end of line while typing on that line. |
|
|
autocmd InsertEnter * match ForbiddenWhitespace /\t\|\s\+\%#\@<!$/ |
|
|
</pre></div> |
|
|
|
|
|
</div> |
|
|
<div class="subsection-level-extent" id="Emacs-configuration"> |
|
|
<h4 class="subsection">2.2.2 Emacs configuration</h4> |
|
|
<p>For Emacs, add these roughly equivalent lines to your <samp class="file">.emacs.d/init.el</samp>: |
|
|
</p><div class="example lisp"> |
|
|
<pre class="lisp-preformatted">(c-add-style "ffmpeg" |
|
|
'("k&r" |
|
|
(c-basic-offset . 4) |
|
|
(indent-tabs-mode . nil) |
|
|
(show-trailing-whitespace . t) |
|
|
(c-offsets-alist |
|
|
(statement-cont . (c-lineup-assignments +))) |
|
|
) |
|
|
) |
|
|
(setq c-default-style "ffmpeg") |
|
|
</pre></div> |
|
|
|
|
|
</div> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Comments"> |
|
|
<h3 class="section">2.3 Comments</h3> |
|
|
<p>Use the JavaDoc/Doxygen format (see examples below) so that code documentation |
|
|
can be generated automatically. All nontrivial functions should have a comment |
|
|
above them explaining what the function does, even if it is just one sentence. |
|
|
All structures and their member variables should be documented, too. |
|
|
</p> |
|
|
<p>Avoid Qt-style and similar Doxygen syntax with <code class="code">!</code> in it, i.e. replace |
|
|
<code class="code">//!</code> with <code class="code">///</code> and similar. Also @ syntax should be employed |
|
|
for markup commands, i.e. use <code class="code">@param</code> and not <code class="code">\param</code>. |
|
|
</p> |
|
|
<div class="example"> |
|
|
<pre class="example-preformatted">/** |
|
|
* @file |
|
|
* MPEG codec. |
|
|
* @author ... |
|
|
*/ |
|
|
|
|
|
/** |
|
|
* Summary sentence. |
|
|
* more text ... |
|
|
* ... |
|
|
*/ |
|
|
typedef struct Foobar { |
|
|
int var1; /**< var1 description */ |
|
|
int var2; ///< var2 description |
|
|
/** var3 description */ |
|
|
int var3; |
|
|
} Foobar; |
|
|
|
|
|
/** |
|
|
* Summary sentence. |
|
|
* more text ... |
|
|
* ... |
|
|
* @param my_parameter description of my_parameter |
|
|
* @return return value description |
|
|
*/ |
|
|
int myfunc(int my_parameter) |
|
|
... |
|
|
</pre></div> |
|
|
|
|
|
<a class="anchor" id="Naming-conventions"></a></div> |
|
|
<div class="section-level-extent" id="Naming-conventions-1"> |
|
|
<h3 class="section">2.4 Naming conventions</h3> |
|
|
|
|
|
<p>Names of functions, variables, and struct members must be lowercase, using |
|
|
underscores (_) to separate words. For example, ‘<samp class="samp">avfilter_get_video_buffer</samp>’ |
|
|
is an acceptable function name and ‘<samp class="samp">AVFilterGetVideo</samp>’ is not. |
|
|
</p> |
|
|
<p>Struct, union, enum, and typedeffed type names must use CamelCase. All structs |
|
|
and unions should be typedeffed to the same name as the struct/union tag, e.g. |
|
|
<code class="code">typedef struct AVFoo { ... } AVFoo;</code>. Enums are typically not |
|
|
typedeffed. |
|
|
</p> |
|
|
<p>Enumeration constants and macros must be UPPERCASE, except for macros |
|
|
masquerading as functions, which should use the function naming convention. |
|
|
</p> |
|
|
<p>All identifiers in the libraries should be namespaced as follows: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>No namespacing for identifiers with file and lower scope (e.g. local variables, |
|
|
static functions), and struct and union members, |
|
|
|
|
|
</li><li>The <code class="code">ff_</code> prefix must be used for variables and functions visible outside |
|
|
of file scope, but only used internally within a single library, e.g. |
|
|
‘<samp class="samp">ff_w64_demuxer</samp>’. This prevents name collisions when FFmpeg is statically |
|
|
linked. |
|
|
|
|
|
</li><li>For variables and functions visible outside of file scope, used internally |
|
|
across multiple libraries, use <code class="code">avpriv_</code> as prefix, for example, |
|
|
‘<samp class="samp">avpriv_report_missing_feature</samp>’. |
|
|
|
|
|
</li><li>All other internal identifiers, like private type or macro names, should be |
|
|
namespaced only to avoid possible internal conflicts. E.g. <code class="code">H264_NAL_SPS</code> |
|
|
vs. <code class="code">HEVC_NAL_SPS</code>. |
|
|
|
|
|
</li><li>Each library has its own prefix for public symbols, in addition to the |
|
|
commonly used <code class="code">av_</code> (<code class="code">avformat_</code> for libavformat, |
|
|
<code class="code">avcodec_</code> for libavcodec, <code class="code">swr_</code> for libswresample, etc). |
|
|
Check the existing code and choose names accordingly. |
|
|
|
|
|
</li><li>Other public identifiers (struct, union, enum, macro, type names) must use their |
|
|
library’s public prefix (<code class="code">AV</code>, <code class="code">Sws</code>, or <code class="code">Swr</code>). |
|
|
</li></ul> |
|
|
|
|
|
<p>Furthermore, name space reserved for the system should not be invaded. |
|
|
Identifiers ending in <code class="code">_t</code> are reserved by |
|
|
<a class="url" href="http://pubs.opengroup.org/onlinepubs/007904975/functions/xsh_chap02_02.html#tag_02_02_02">POSIX</a>. |
|
|
Also avoid names starting with <code class="code">__</code> or <code class="code">_</code> followed by an uppercase |
|
|
letter as they are reserved by the C standard. Names starting with <code class="code">_</code> |
|
|
are reserved at the file level and may not be used for externally visible |
|
|
symbols. If in doubt, just avoid names starting with <code class="code">_</code> altogether. |
|
|
</p> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Miscellaneous-conventions"> |
|
|
<h3 class="section">2.5 Miscellaneous conventions</h3> |
|
|
|
|
|
<ul class="itemize mark-bullet"> |
|
|
<li>fprintf and printf are forbidden in libavformat and libavcodec, |
|
|
please use av_log() instead. |
|
|
|
|
|
</li><li>Casts should be used only when necessary. Unneeded parentheses |
|
|
should also be avoided if they don’t make the code easier to understand. |
|
|
</li></ul> |
|
|
|
|
|
<a class="anchor" id="Development-Policy"></a></div> |
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Development-Policy-1"> |
|
|
<h2 class="chapter">3 Development Policy</h2> |
|
|
|
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#Patches_002fCommitting" accesskey="1">Patches/Committing</a></li> |
|
|
<li><a href="#Code" accesskey="2">Code</a></li> |
|
|
<li><a href="#Library-public-interfaces" accesskey="3">Library public interfaces</a></li> |
|
|
<li><a href="#Documentation_002fOther" accesskey="4">Documentation/Other</a></li> |
|
|
</ul> |
|
|
<div class="section-level-extent" id="Patches_002fCommitting"> |
|
|
<h3 class="section">3.1 Patches/Committing</h3> |
|
|
<h4 class="subheading" id="Licenses-for-patches-must-be-compatible-with-FFmpeg_002e">Licenses for patches must be compatible with FFmpeg.</h4> |
|
|
<p>Contributions should be licensed under the |
|
|
<a class="uref" href="http://www.gnu.org/licenses/lgpl-2.1.html">LGPL 2.1</a>, |
|
|
including an "or any later version" clause, or, if you prefer |
|
|
a gift-style license, the |
|
|
<a class="uref" href="http://opensource.org/licenses/isc-license.txt">ISC</a> or |
|
|
<a class="uref" href="http://mit-license.org/">MIT</a> license. |
|
|
<a class="uref" href="http://www.gnu.org/licenses/gpl-2.0.html">GPL 2</a> including |
|
|
an "or any later version" clause is also acceptable, but LGPL is |
|
|
preferred. |
|
|
If you add a new file, give it a proper license header. Do not copy and |
|
|
paste it from a random place, use an existing file as template. |
|
|
</p> |
|
|
<h4 class="subheading" id="You-must-not-commit-code-which-breaks-FFmpeg_0021">You must not commit code which breaks FFmpeg!</h4> |
|
|
<p>This means unfinished code which is enabled and breaks compilation, |
|
|
or compiles but does not work/breaks the regression tests. Code which |
|
|
is unfinished but disabled may be permitted under-circumstances, like |
|
|
missing samples or an implementation with a small subset of features. |
|
|
Always check the mailing list for any reviewers with issues and test |
|
|
FATE before you push. |
|
|
</p> |
|
|
<h4 class="subheading" id="Keep-the-main-commit-message-short-with-an-extended-description-below_002e">Keep the main commit message short with an extended description below.</h4> |
|
|
<p>The commit message should have a short first line in the form of |
|
|
a ‘<samp class="samp">topic: short description</samp>’ as a header, separated by a newline |
|
|
from the body consisting of an explanation of why the change is necessary. |
|
|
If the commit fixes a known bug on the bug tracker, the commit message |
|
|
should include its bug ID. Referring to the issue on the bug tracker does |
|
|
not exempt you from writing an excerpt of the bug in the commit message. |
|
|
</p> |
|
|
<h4 class="subheading" id="Testing-must-be-adequate-but-not-excessive_002e">Testing must be adequate but not excessive.</h4> |
|
|
<p>If it works for you, others, and passes FATE then it should be OK to commit |
|
|
it, provided it fits the other committing criteria. You should not worry about |
|
|
over-testing things. If your code has problems (portability, triggers |
|
|
compiler bugs, unusual environment etc) they will be reported and eventually |
|
|
fixed. |
|
|
</p> |
|
|
<h4 class="subheading" id="Do-not-commit-unrelated-changes-together_002e">Do not commit unrelated changes together.</h4> |
|
|
<p>They should be split them into self-contained pieces. Also do not forget |
|
|
that if part B depends on part A, but A does not depend on B, then A can |
|
|
and should be committed first and separate from B. Keeping changes well |
|
|
split into self-contained parts makes reviewing and understanding them on |
|
|
the commit log mailing list easier. This also helps in case of debugging |
|
|
later on. |
|
|
Also if you have doubts about splitting or not splitting, do not hesitate to |
|
|
ask/discuss it on the developer mailing list. |
|
|
</p> |
|
|
<h4 class="subheading" id="Ask-before-you-change-the-build-system-_0028configure_002c-etc_0029_002e">Ask before you change the build system (configure, etc).</h4> |
|
|
<p>Do not commit changes to the build system (Makefiles, configure script) |
|
|
which change behavior, defaults etc, without asking first. The same |
|
|
applies to compiler warning fixes, trivial looking fixes and to code |
|
|
maintained by other developers. We usually have a reason for doing things |
|
|
the way we do. Send your changes as patches to the ffmpeg-devel mailing |
|
|
list, and if the code maintainers say OK, you may commit. This does not |
|
|
apply to files you wrote and/or maintain. |
|
|
</p> |
|
|
<h4 class="subheading" id="Cosmetic-changes-should-be-kept-in-separate-patches_002e">Cosmetic changes should be kept in separate patches.</h4> |
|
|
<p>We refuse source indentation and other cosmetic changes if they are mixed |
|
|
with functional changes, such commits will be rejected and removed. Every |
|
|
developer has his own indentation style, you should not change it. Of course |
|
|
if you (re)write something, you can use your own style, even though we would |
|
|
prefer if the indentation throughout FFmpeg was consistent (Many projects |
|
|
force a given indentation style - we do not.). If you really need to make |
|
|
indentation changes (try to avoid this), separate them strictly from real |
|
|
changes. |
|
|
</p> |
|
|
<p>NOTE: If you had to put if(){ .. } over a large (> 5 lines) chunk of code, |
|
|
then either do NOT change the indentation of the inner part within (do not |
|
|
move it to the right)! or do so in a separate commit |
|
|
</p> |
|
|
<h4 class="subheading" id="Commit-messages-should-always-be-filled-out-properly_002e">Commit messages should always be filled out properly.</h4> |
|
|
<p>Always fill out the commit log message. Describe in a few lines what you |
|
|
changed and why. You can refer to mailing list postings if you fix a |
|
|
particular bug. Comments such as "fixed!" or "Changed it." are unacceptable. |
|
|
Recommended format: |
|
|
</p> |
|
|
<div class="example"> |
|
|
<pre class="example-preformatted">area changed: Short 1 line description |
|
|
|
|
|
details describing what and why and giving references. |
|
|
</pre></div> |
|
|
|
|
|
<h4 class="subheading" id="Credit-the-author-of-the-patch_002e">Credit the author of the patch.</h4> |
|
|
<p>Make sure the author of the commit is set correctly. (see git commit –author) |
|
|
If you apply a patch, send an |
|
|
answer to ffmpeg-devel (or wherever you got the patch from) saying that |
|
|
you applied the patch. |
|
|
</p> |
|
|
<h4 class="subheading" id="Complex-patches-should-refer-to-discussion-surrounding-them_002e">Complex patches should refer to discussion surrounding them.</h4> |
|
|
<p>When applying patches that have been discussed (at length) on the mailing |
|
|
list, reference the thread in the log message. |
|
|
</p> |
|
|
<h4 class="subheading" id="Always-wait-long-enough-before-pushing-changes">Always wait long enough before pushing changes</h4> |
|
|
<p>Do NOT commit to code actively maintained by others without permission. |
|
|
Send a patch to ffmpeg-devel. If no one answers within a reasonable |
|
|
time-frame (12h for build failures and security fixes, 3 days small changes, |
|
|
1 week for big patches) then commit your patch if you think it is OK. |
|
|
Also note, the maintainer can simply ask for more time to review! |
|
|
</p> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Code"> |
|
|
<h3 class="section">3.2 Code</h3> |
|
|
<h4 class="subheading" id="Warnings-for-correct-code-may-be-disabled-if-there-is-no-other-option_002e">Warnings for correct code may be disabled if there is no other option.</h4> |
|
|
<p>Compiler warnings indicate potential bugs or code with bad style. If a type of |
|
|
warning always points to correct and clean code, that warning should |
|
|
be disabled, not the code changed. |
|
|
Thus the remaining warnings can either be bugs or correct code. |
|
|
If it is a bug, the bug has to be fixed. If it is not, the code should |
|
|
be changed to not generate a warning unless that causes a slowdown |
|
|
or obfuscates the code. |
|
|
</p> |
|
|
<h4 class="subheading" id="Check-untrusted-input-properly_002e">Check untrusted input properly.</h4> |
|
|
<p>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. |
|
|
</p> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Library-public-interfaces"> |
|
|
<h3 class="section">3.3 Library public interfaces</h3> |
|
|
<p>Every library in FFmpeg provides a set of public APIs in its installed headers, |
|
|
which are those listed in the variable <code class="code">HEADERS</code> in that library’s |
|
|
<samp class="file">Makefile</samp>. All identifiers defined in those headers (except for those |
|
|
explicitly documented otherwise), and corresponding symbols exported from |
|
|
compiled shared or static libraries are considered public interfaces and must |
|
|
comply with the API and ABI compatibility rules described in this section. |
|
|
</p> |
|
|
<p>Public APIs must be backward compatible within a given major version. I.e. any |
|
|
valid user code that compiles and works with a given library version must still |
|
|
compile and work with any later version, as long as the major version number is |
|
|
unchanged. "Valid user code" here means code that is calling our APIs in a |
|
|
documented and/or intended manner and is not relying on any undefined behavior. |
|
|
Incrementing the major version may break backward compatibility, but only to the |
|
|
extent described in <a class="ref" href="#Major-version-bumps">Major version bumps</a>. |
|
|
</p> |
|
|
<p>We also guarantee backward ABI compatibility for shared and static libraries. |
|
|
I.e. it should be possible to replace a shared or static build of our library |
|
|
with a build of any later version (re-linking the user binary in the static |
|
|
case) without breaking any valid user binaries, as long as the major version |
|
|
number remains unchanged. |
|
|
</p> |
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#Adding-new-interfaces" accesskey="1">Adding new interfaces</a></li> |
|
|
<li><a href="#Removing-interfaces-1" accesskey="2">Removing interfaces</a></li> |
|
|
<li><a href="#Major-version-bumps-1" accesskey="3">Major version bumps</a></li> |
|
|
</ul> |
|
|
<div class="subsection-level-extent" id="Adding-new-interfaces"> |
|
|
<h4 class="subsection">3.3.1 Adding new interfaces</h4> |
|
|
<p>Any new public identifiers in installed headers are considered new API - this |
|
|
includes new functions, structs, macros, enum values, typedefs, new fields in |
|
|
existing functions, new installed headers, etc. Consider the following |
|
|
guidelines when adding new APIs. |
|
|
</p> |
|
|
<h4 class="subsubheading" id="Motivation">Motivation</h4> |
|
|
<p>While new APIs can be added relatively easily, changing or removing them is much |
|
|
harder due to abovementioned compatibility requirements. You should then |
|
|
consider carefully whether the functionality you are adding really needs to be |
|
|
exposed to our callers as new public API. |
|
|
</p> |
|
|
<p>Your new API should have at least one well-established use case outside of the |
|
|
library that cannot be easily achieved with existing APIs. Every library in |
|
|
FFmpeg also has a defined scope - your new API must fit within it. |
|
|
</p> |
|
|
<h4 class="subsubheading" id="Replacing-existing-APIs">Replacing existing APIs</h4> |
|
|
<p>If your new API is replacing an existing one, it should be strictly superior to |
|
|
it, so that the advantages of using the new API outweight the cost to the |
|
|
callers of changing their code. After adding the new API you should then |
|
|
deprecate the old one and schedule it for removal, as described in |
|
|
<a class="ref" href="#Removing-interfaces">Removing interfaces</a>. |
|
|
</p> |
|
|
<p>If you deem an existing API deficient and want to fix it, the preferred approach |
|
|
in most cases is to add a differently-named replacement and deprecate the |
|
|
existing API rather than modify it. It is important to make the changes visible |
|
|
to our callers (e.g. through compile- or run-time deprecation warnings) and make |
|
|
it clear how to transition to the new API (e.g. in the Doxygen documentation or |
|
|
on the wiki). |
|
|
</p> |
|
|
<h4 class="subsubheading" id="API-design">API design</h4> |
|
|
<p>The FFmpeg libraries are used by a variety of callers to perform a wide range of |
|
|
multimedia-related processing tasks. You should therefore - within reason - try |
|
|
to design your new API for the broadest feasible set of use cases and avoid |
|
|
unnecessarily limiting it to a specific type of callers (e.g. just media |
|
|
playback or just transcoding). |
|
|
</p> |
|
|
<h4 class="subsubheading" id="Consistency">Consistency</h4> |
|
|
<p>Check whether similar APIs already exist in FFmpeg. If they do, try to model |
|
|
your new addition on them to achieve better overall consistency. |
|
|
</p> |
|
|
<p>The naming of your new identifiers should follow the <a class="ref" href="#Naming-conventions">Naming conventions</a> |
|
|
and be aligned with other similar APIs, if applicable. |
|
|
</p> |
|
|
<h4 class="subsubheading" id="Extensibility">Extensibility</h4> |
|
|
<p>You should also consider how your API might be extended in the future in a |
|
|
backward-compatible way. If you are adding a new struct <code class="code">AVFoo</code>, the |
|
|
standard approach is requiring the caller to always allocate it through a |
|
|
constructor function, typically named <code class="code">av_foo_alloc()</code>. This way new fields |
|
|
may be added to the end of the struct without breaking ABI compatibility. |
|
|
Typically you will also want a destructor - <code class="code">av_foo_free(AVFoo**)</code> that |
|
|
frees the indirectly supplied object (and its contents, if applicable) and |
|
|
writes <code class="code">NULL</code> to the supplied pointer, thus eliminating the potential |
|
|
dangling pointer in the caller’s memory. |
|
|
</p> |
|
|
<p>If you are adding new functions, consider whether it might be desirable to tweak |
|
|
their behavior in the future - you may want to add a flags argument, even though |
|
|
it would be unused initially. |
|
|
</p> |
|
|
<h4 class="subsubheading" id="Documentation">Documentation</h4> |
|
|
<p>All new APIs must be documented as Doxygen-formatted comments above the |
|
|
identifiers you add to the public headers. You should also briefly mention the |
|
|
change in <samp class="file">doc/APIchanges</samp>. |
|
|
</p> |
|
|
<h4 class="subsubheading" id="Bump-the-version">Bump the version</h4> |
|
|
<p>Backward-incompatible API or ABI changes require incrementing (bumping) the |
|
|
major version number, as described in <a class="ref" href="#Major-version-bumps">Major version bumps</a>. Major |
|
|
bumps are significant events that happen on a schedule - so if your change |
|
|
strictly requires one you should add it under <code class="code">#if</code> preprocesor guards that |
|
|
disable it until the next major bump happens. |
|
|
</p> |
|
|
<p>New APIs that can be added without breaking API or ABI compatibility require |
|
|
bumping the minor version number. |
|
|
</p> |
|
|
<p>Incrementing the third (micro) version component means a noteworthy binary |
|
|
compatible change (e.g. encoder bug fix that matters for the decoder). The third |
|
|
component always starts at 100 to distinguish FFmpeg from Libav. |
|
|
</p> |
|
|
<a class="anchor" id="Removing-interfaces"></a></div> |
|
|
<div class="subsection-level-extent" id="Removing-interfaces-1"> |
|
|
<h4 class="subsection">3.3.2 Removing interfaces</h4> |
|
|
<p>Due to abovementioned compatibility guarantees, removing APIs is an involved |
|
|
process that should only be undertaken with good reason. Typically a deficient, |
|
|
restrictive, or otherwise inadequate API is replaced by a superior one, though |
|
|
it does at times happen that we remove an API without any replacement (e.g. when |
|
|
the feature it provides is deemed not worth the maintenance effort, out of scope |
|
|
of the project, fundamentally flawed, etc.). |
|
|
</p> |
|
|
<p>The removal has two steps - first the API is deprecated and scheduled for |
|
|
removal, but remains present and functional. The second step is actually |
|
|
removing the API - this is described in <a class="ref" href="#Major-version-bumps">Major version bumps</a>. |
|
|
</p> |
|
|
<p>To deprecate an API you should signal to our users that they should stop using |
|
|
it. E.g. if you intend to remove struct members or functions, you should mark |
|
|
them with <code class="code">attribute_deprecated</code>. When this cannot be done, it may be |
|
|
possible to detect the use of the deprecated API at runtime and print a warning |
|
|
(though take care not to print it too often). You should also document the |
|
|
deprecation (and the replacement, if applicable) in the relevant Doxygen |
|
|
documentation block. |
|
|
</p> |
|
|
<p>Finally, you should define a deprecation guard along the lines of |
|
|
<code class="code">#define FF_API_<FOO> (LIBAVBAR_VERSION_MAJOR < XX)</code> (where XX is the major |
|
|
version in which the API will be removed) in <samp class="file">libavbar/version_major.h</samp> |
|
|
(<samp class="file">version.h</samp> in case of <code class="code">libavutil</code>). Then wrap all uses of the |
|
|
deprecated API in <code class="code">#if FF_API_<FOO> .... #endif</code>, so that the code will |
|
|
automatically get disabled once the major version reaches XX. You can also use |
|
|
<code class="code">FF_DISABLE_DEPRECATION_WARNINGS</code> and <code class="code">FF_ENABLE_DEPRECATION_WARNINGS</code> |
|
|
to suppress compiler deprecation warnings inside these guards. You should test |
|
|
that the code compiles and works with the guard macro evaluating to both true |
|
|
and false. |
|
|
</p> |
|
|
<a class="anchor" id="Major-version-bumps"></a></div> |
|
|
<div class="subsection-level-extent" id="Major-version-bumps-1"> |
|
|
<h4 class="subsection">3.3.3 Major version bumps</h4> |
|
|
<p>A major version bump signifies an API and/or ABI compatibility break. To reduce |
|
|
the negative effects on our callers, who are required to adapt their code, |
|
|
backward-incompatible changes during a major bump should be limited to: |
|
|
</p><ul class="itemize mark-bullet"> |
|
|
<li>Removing previously deprecated APIs. |
|
|
|
|
|
</li><li>Performing ABI- but not API-breaking changes, like reordering struct contents. |
|
|
</li></ul> |
|
|
|
|
|
</div> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Documentation_002fOther"> |
|
|
<h3 class="section">3.4 Documentation/Other</h3> |
|
|
<h4 class="subheading" id="Subscribe-to-the-ffmpeg_002ddevel-mailing-list_002e">Subscribe to the ffmpeg-devel mailing list.</h4> |
|
|
<p>It is important to be subscribed to the |
|
|
<a class="uref" href="https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel">ffmpeg-devel</a> |
|
|
mailing list. Almost any non-trivial patch is to be sent there for review. |
|
|
Other developers may have comments about your contribution. We expect you see |
|
|
those comments, and to improve it if requested. (N.B. Experienced committers |
|
|
have other channels, and may sometimes skip review for trivial fixes.) Also, |
|
|
discussion here about bug fixes and FFmpeg improvements by other developers may |
|
|
be helpful information for you. Finally, by being a list subscriber, your |
|
|
contribution will be posted immediately to the list, without the moderation |
|
|
hold which messages from non-subscribers experience. |
|
|
</p> |
|
|
<p>However, it is more important to the project that we receive your patch than |
|
|
that you be subscribed to the ffmpeg-devel list. If you have a patch, and don’t |
|
|
want to subscribe and discuss the patch, then please do send it to the list |
|
|
anyway. |
|
|
</p> |
|
|
<h4 class="subheading" id="Subscribe-to-the-ffmpeg_002dcvslog-mailing-list_002e">Subscribe to the ffmpeg-cvslog mailing list.</h4> |
|
|
<p>Diffs of all commits are sent to the |
|
|
<a class="uref" href="https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-cvslog">ffmpeg-cvslog</a> |
|
|
mailing list. Some developers read this list to review all code base changes |
|
|
from all sources. Subscribing to this list is not mandatory. |
|
|
</p> |
|
|
<h4 class="subheading" id="Keep-the-documentation-up-to-date_002e">Keep the documentation up to date.</h4> |
|
|
<p>Update the documentation if you change behavior or add features. If you are |
|
|
unsure how best to do this, send a patch to ffmpeg-devel, the documentation |
|
|
maintainer(s) will review and commit your stuff. |
|
|
</p> |
|
|
<h4 class="subheading" id="Important-discussions-should-be-accessible-to-all_002e">Important discussions should be accessible to all.</h4> |
|
|
<p>Try to keep important discussions and requests (also) on the public |
|
|
developer mailing list, so that all developers can benefit from them. |
|
|
</p> |
|
|
<h4 class="subheading" id="Check-your-entries-in-MAINTAINERS_002e">Check your entries in MAINTAINERS.</h4> |
|
|
<p>Make sure that no parts of the codebase that you maintain are missing from the |
|
|
<samp class="file">MAINTAINERS</samp> file. If something that you want to maintain is missing add it with |
|
|
your name after it. |
|
|
If at some point you no longer want to maintain some code, then please help in |
|
|
finding a new maintainer and also don’t forget to update the <samp class="file">MAINTAINERS</samp> file. |
|
|
</p> |
|
|
<p>We think our rules are not too hard. If you have comments, contact us. |
|
|
</p> |
|
|
<a class="anchor" id="Submitting-patches"></a></div> |
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Submitting-patches-1"> |
|
|
<h2 class="chapter">4 Submitting patches</h2> |
|
|
|
|
|
<p>First, read the <a class="ref" href="#Coding-Rules">Coding Rules</a> above if you did not yet, in particular |
|
|
the rules regarding patch submission. |
|
|
</p> |
|
|
<p>When you submit your patch, please use <code class="code">git format-patch</code> or |
|
|
<code class="code">git send-email</code>. We cannot read other diffs :-). |
|
|
</p> |
|
|
<p>Also please do not submit a patch which contains several unrelated changes. |
|
|
Split it into separate, self-contained pieces. This does not mean splitting |
|
|
file by file. Instead, make the patch as small as possible while still |
|
|
keeping it as a logical unit that contains an individual change, even |
|
|
if it spans multiple files. This makes reviewing your patches much easier |
|
|
for us and greatly increases your chances of getting your patch applied. |
|
|
</p> |
|
|
<p>Use the patcheck tool of FFmpeg to check your patch. |
|
|
The tool is located in the tools directory. |
|
|
</p> |
|
|
<p>Run the <a class="ref" href="#Regression-tests">Regression tests</a> before submitting a patch in order to verify |
|
|
it does not cause unexpected problems. |
|
|
</p> |
|
|
<p>It also helps quite a bit if you tell us what the patch does (for example |
|
|
’replaces lrint by lrintf’), and why (for example ’*BSD isn’t C99 compliant |
|
|
and has no lrint()’) |
|
|
</p> |
|
|
<p>Also please if you send several patches, send each patch as a separate mail, |
|
|
do not attach several unrelated patches to the same mail. |
|
|
</p> |
|
|
<p>Patches should be posted to the |
|
|
<a class="uref" href="https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel">ffmpeg-devel</a> |
|
|
mailing list. Use <code class="code">git send-email</code> when possible since it will properly |
|
|
send patches without requiring extra care. If you cannot, then send patches |
|
|
as base64-encoded attachments, so your patch is not trashed during |
|
|
transmission. Also ensure the correct mime type is used |
|
|
(text/x-diff or text/x-patch or at least text/plain) and that only one |
|
|
patch is inline or attached per mail. |
|
|
You can check <a class="url" href="https://patchwork.ffmpeg.org">https://patchwork.ffmpeg.org</a>, if your patch does not show up, its mime type |
|
|
likely was wrong. |
|
|
</p> |
|
|
<h4 class="subheading" id="Sending-patches-from-email-clients">Sending patches from email clients</h4> |
|
|
<p>Using <code class="code">git send-email</code> might not be desirable for everyone. The |
|
|
following trick allows to send patches via email clients in a safe |
|
|
way. It has been tested with Outlook and Thunderbird (with X-Unsent |
|
|
extension) and might work with other applications. |
|
|
</p> |
|
|
<p>Create your patch like this: |
|
|
</p> |
|
|
<pre class="verbatim">git format-patch -s -o "outputfolder" --add-header "X-Unsent: 1" --suffix .eml --to ffmpeg-devel@ffmpeg.org -1 1a2b3c4d |
|
|
</pre> |
|
|
<p>Now you’ll just need to open the eml file with the email application |
|
|
and execute ’Send’. |
|
|
</p> |
|
|
<h4 class="subheading" id="Reviews">Reviews</h4> |
|
|
<p>Your patch will be reviewed on the mailing list. You will likely be asked |
|
|
to make some changes and are expected to send in an improved version that |
|
|
incorporates the requests from the review. This process may go through |
|
|
several iterations. Once your patch is deemed good enough, some developer |
|
|
will pick it up and commit it to the official FFmpeg tree. |
|
|
</p> |
|
|
<p>Give us a few days to react. But if some time passes without reaction, |
|
|
send a reminder by email. Your patch should eventually be dealt with. |
|
|
</p> |
|
|
|
|
|
</div> |
|
|
<div class="chapter-level-extent" id="New-codecs-or-formats-checklist"> |
|
|
<h2 class="chapter">5 New codecs or formats checklist</h2> |
|
|
|
|
|
<ol class="enumerate"> |
|
|
<li> Did you use av_cold for codec initialization and close functions? |
|
|
|
|
|
</li><li> Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or |
|
|
AVInputFormat/AVOutputFormat struct? |
|
|
|
|
|
</li><li> Did you bump the minor version number (and reset the micro version |
|
|
number) in <samp class="file">libavcodec/version.h</samp> or <samp class="file">libavformat/version.h</samp>? |
|
|
|
|
|
</li><li> Did you register it in <samp class="file">allcodecs.c</samp> or <samp class="file">allformats.c</samp>? |
|
|
|
|
|
</li><li> Did you add the AVCodecID to <samp class="file">avcodec.h</samp>? |
|
|
When adding new codec IDs, also add an entry to the codec descriptor |
|
|
list in <samp class="file">libavcodec/codec_desc.c</samp>. |
|
|
|
|
|
</li><li> If it has a FourCC, did you add it to <samp class="file">libavformat/riff.c</samp>, |
|
|
even if it is only a decoder? |
|
|
|
|
|
</li><li> Did you add a rule to compile the appropriate files in the Makefile? |
|
|
Remember to do this even if you’re just adding a format to a file that is |
|
|
already being compiled by some other rule, like a raw demuxer. |
|
|
|
|
|
</li><li> Did you add an entry to the table of supported formats or codecs in |
|
|
<samp class="file">doc/general.texi</samp>? |
|
|
|
|
|
</li><li> Did you add an entry in the Changelog? |
|
|
|
|
|
</li><li> If it depends on a parser or a library, did you add that dependency in |
|
|
configure? |
|
|
|
|
|
</li><li> Did you <code class="code">git add</code> the appropriate files before committing? |
|
|
|
|
|
</li><li> Did you make sure it compiles standalone, i.e. with |
|
|
<code class="code">configure --disable-everything --enable-decoder=foo</code> |
|
|
(or <code class="code">--enable-demuxer</code> or whatever your component is)? |
|
|
</li></ol> |
|
|
|
|
|
|
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Patch-submission-checklist"> |
|
|
<h2 class="chapter">6 Patch submission checklist</h2> |
|
|
|
|
|
<ol class="enumerate"> |
|
|
<li> Does <code class="code">make fate</code> pass with the patch applied? |
|
|
|
|
|
</li><li> Was the patch generated with git format-patch or send-email? |
|
|
|
|
|
</li><li> Did you sign-off your patch? (<code class="code">git commit -s</code>) |
|
|
See <a class="uref" href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/Documentation/process/submitting-patches.rst">Sign your work</a> for the meaning |
|
|
of <em class="dfn">sign-off</em>. |
|
|
|
|
|
</li><li> Did you provide a clear git commit log message? |
|
|
|
|
|
</li><li> Is the patch against latest FFmpeg git master branch? |
|
|
|
|
|
</li><li> Are you subscribed to ffmpeg-devel? |
|
|
(the list is subscribers only due to spam) |
|
|
|
|
|
</li><li> Have you checked that the changes are minimal, so that the same cannot be |
|
|
achieved with a smaller patch and/or simpler final code? |
|
|
|
|
|
</li><li> If the change is to speed critical code, did you benchmark it? |
|
|
|
|
|
</li><li> If you did any benchmarks, did you provide them in the mail? |
|
|
|
|
|
</li><li> Have you checked that the patch does not introduce buffer overflows or |
|
|
other security issues? |
|
|
|
|
|
</li><li> Did you test your decoder or demuxer against damaged data? If no, see |
|
|
tools/trasher, the noise bitstream filter, and |
|
|
<a class="uref" href="http://caca.zoy.org/wiki/zzuf">zzuf</a>. Your decoder or demuxer |
|
|
should not crash, end in a (near) infinite loop, or allocate ridiculous |
|
|
amounts of memory when fed damaged data. |
|
|
|
|
|
</li><li> Did you test your decoder or demuxer against sample files? |
|
|
Samples may be obtained at <a class="url" href="https://samples.ffmpeg.org">https://samples.ffmpeg.org</a>. |
|
|
|
|
|
</li><li> Does the patch not mix functional and cosmetic changes? |
|
|
|
|
|
</li><li> Did you add tabs or trailing whitespace to the code? Both are forbidden. |
|
|
|
|
|
</li><li> Is the patch attached to the email you send? |
|
|
|
|
|
</li><li> Is the mime type of the patch correct? It should be text/x-diff or |
|
|
text/x-patch or at least text/plain and not application/octet-stream. |
|
|
|
|
|
</li><li> If the patch fixes a bug, did you provide a verbose analysis of the bug? |
|
|
|
|
|
</li><li> If the patch fixes a bug, did you provide enough information, including |
|
|
a sample, so the bug can be reproduced and the fix can be verified? |
|
|
Note please do not attach samples >100k to mails but rather provide a |
|
|
URL, you can upload to <a class="url" href="https://streams.videolan.org/upload/">https://streams.videolan.org/upload/</a>. |
|
|
|
|
|
</li><li> Did you provide a verbose summary about what the patch does change? |
|
|
|
|
|
</li><li> Did you provide a verbose explanation why it changes things like it does? |
|
|
|
|
|
</li><li> Did you provide a verbose summary of the user visible advantages and |
|
|
disadvantages if the patch is applied? |
|
|
|
|
|
</li><li> Did you provide an example so we can verify the new feature added by the |
|
|
patch easily? |
|
|
|
|
|
</li><li> If you added a new file, did you insert a license header? It should be |
|
|
taken from FFmpeg, not randomly copied and pasted from somewhere else. |
|
|
|
|
|
</li><li> You should maintain alphabetical order in alphabetically ordered lists as |
|
|
long as doing so does not break API/ABI compatibility. |
|
|
|
|
|
</li><li> Lines with similar content should be aligned vertically when doing so |
|
|
improves readability. |
|
|
|
|
|
</li><li> Consider adding a regression test for your code. |
|
|
|
|
|
</li><li> If you added YASM code please check that things still work with –disable-yasm. |
|
|
|
|
|
</li><li> Make sure you check the return values of function and return appropriate |
|
|
error codes. Especially memory allocation functions like <code class="code">av_malloc()</code> |
|
|
are notoriously left unchecked, which is a serious problem. |
|
|
|
|
|
</li><li> Test your code with valgrind and or Address Sanitizer to ensure it’s free |
|
|
of leaks, out of array accesses, etc. |
|
|
</li></ol> |
|
|
|
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Patch-review-process"> |
|
|
<h2 class="chapter">7 Patch review process</h2> |
|
|
|
|
|
<p>All patches posted to ffmpeg-devel will be reviewed, unless they contain a |
|
|
clear note that the patch is not for the git master branch. |
|
|
Reviews and comments will be posted as replies to the patch on the |
|
|
mailing list. The patch submitter then has to take care of every comment, |
|
|
that can be by resubmitting a changed patch or by discussion. Resubmitted |
|
|
patches will themselves be reviewed like any other patch. If at some point |
|
|
a patch passes review with no comments then it is approved, that can for |
|
|
simple and small patches happen immediately while large patches will generally |
|
|
have to be changed and reviewed many times before they are approved. |
|
|
After a patch is approved it will be committed to the repository. |
|
|
</p> |
|
|
<p>We will review all submitted patches, but sometimes we are quite busy so |
|
|
especially for large patches this can take several weeks. |
|
|
</p> |
|
|
<p>If you feel that the review process is too slow and you are willing to try to |
|
|
take over maintainership of the area of code you change then just clone |
|
|
git master and maintain the area of code there. We will merge each area from |
|
|
where its best maintained. |
|
|
</p> |
|
|
<p>When resubmitting patches, please do not make any significant changes |
|
|
not related to the comments received during review. Such patches will |
|
|
be rejected. Instead, submit significant changes or new features as |
|
|
separate patches. |
|
|
</p> |
|
|
<p>Everyone is welcome to review patches. Also if you are waiting for your patch |
|
|
to be reviewed, please consider helping to review other patches, that is a great |
|
|
way to get everyone’s patches reviewed sooner. |
|
|
</p> |
|
|
<a class="anchor" id="Regression-tests"></a></div> |
|
|
<div class="chapter-level-extent" id="Regression-tests-1"> |
|
|
<h2 class="chapter">8 Regression tests</h2> |
|
|
|
|
|
<p>Before submitting a patch (or committing to the repository), you should at least |
|
|
test that you did not break anything. |
|
|
</p> |
|
|
<p>Running ’make fate’ accomplishes this, please see <a class="url" href="fate.html">fate.html</a> for details. |
|
|
</p> |
|
|
<p>[Of course, some patches may change the results of the regression tests. In |
|
|
this case, the reference results of the regression tests shall be modified |
|
|
accordingly]. |
|
|
</p> |
|
|
<ul class="mini-toc"> |
|
|
<li><a href="#Adding-files-to-the-fate_002dsuite-dataset" accesskey="1">Adding files to the fate-suite dataset</a></li> |
|
|
<li><a href="#Visualizing-Test-Coverage" accesskey="2">Visualizing Test Coverage</a></li> |
|
|
<li><a href="#Using-Valgrind" accesskey="3">Using Valgrind</a></li> |
|
|
</ul> |
|
|
<div class="section-level-extent" id="Adding-files-to-the-fate_002dsuite-dataset"> |
|
|
<h3 class="section">8.1 Adding files to the fate-suite dataset</h3> |
|
|
|
|
|
<p>If you need a sample uploaded send a mail to samples-request. |
|
|
</p> |
|
|
<p>When there is no muxer or encoder available to generate test media for a |
|
|
specific test then the media has to be included in the fate-suite. |
|
|
First please make sure that the sample file is as small as possible to test the |
|
|
respective decoder or demuxer sufficiently. Large files increase network |
|
|
bandwidth and disk space requirements. |
|
|
Once you have a working fate test and fate sample, provide in the commit |
|
|
message or introductory message for the patch series that you post to |
|
|
the ffmpeg-devel mailing list, a direct link to download the sample media. |
|
|
</p> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Visualizing-Test-Coverage"> |
|
|
<h3 class="section">8.2 Visualizing Test Coverage</h3> |
|
|
|
|
|
<p>The FFmpeg build system allows visualizing the test coverage in an easy |
|
|
manner with the coverage tools <code class="code">gcov</code>/<code class="code">lcov</code>. This involves |
|
|
the following steps: |
|
|
</p> |
|
|
<ol class="enumerate"> |
|
|
<li> Configure to compile with instrumentation enabled: |
|
|
<code class="code">configure --toolchain=gcov</code>. |
|
|
|
|
|
</li><li> Run your test case, either manually or via FATE. This can be either |
|
|
the full FATE regression suite, or any arbitrary invocation of any |
|
|
front-end tool provided by FFmpeg, in any combination. |
|
|
|
|
|
</li><li> Run <code class="code">make lcov</code> to generate coverage data in HTML format. |
|
|
|
|
|
</li><li> View <code class="code">lcov/index.html</code> in your preferred HTML viewer. |
|
|
</li></ol> |
|
|
|
|
|
<p>You can use the command <code class="code">make lcov-reset</code> to reset the coverage |
|
|
measurements. You will need to rerun <code class="code">make lcov</code> after running a |
|
|
new test. |
|
|
</p> |
|
|
</div> |
|
|
<div class="section-level-extent" id="Using-Valgrind"> |
|
|
<h3 class="section">8.3 Using Valgrind</h3> |
|
|
|
|
|
<p>The configure script provides a shortcut for using valgrind to spot bugs |
|
|
related to memory handling. Just add the option |
|
|
<code class="code">--toolchain=valgrind-memcheck</code> or <code class="code">--toolchain=valgrind-massif</code> |
|
|
to your configure line, and reasonable defaults will be set for running |
|
|
FATE under the supervision of either the <strong class="strong">memcheck</strong> or the |
|
|
<strong class="strong">massif</strong> tool of the valgrind suite. |
|
|
</p> |
|
|
<p>In case you need finer control over how valgrind is invoked, use the |
|
|
<code class="code">--target-exec='valgrind <your_custom_valgrind_options></code> option in |
|
|
your configure line instead. |
|
|
</p> |
|
|
<a class="anchor" id="Release-process"></a></div> |
|
|
</div> |
|
|
<div class="chapter-level-extent" id="Release-process-1"> |
|
|
<h2 class="chapter">9 Release process</h2> |
|
|
|
|
|
<p>FFmpeg maintains a set of <strong class="strong">release branches</strong>, which are the |
|
|
recommended deliverable for system integrators and distributors (such as |
|
|
Linux distributions, etc.). At regular times, a <strong class="strong">release |
|
|
manager</strong> prepares, tests and publishes tarballs on the |
|
|
<a class="url" href="https://ffmpeg.org">https://ffmpeg.org</a> website. |
|
|
</p> |
|
|
<p>There are two kinds of releases: |
|
|
</p> |
|
|
<ol class="enumerate"> |
|
|
<li> <strong class="strong">Major releases</strong> always include the latest and greatest |
|
|
features and functionality. |
|
|
|
|
|
</li><li> <strong class="strong">Point releases</strong> are cut from <strong class="strong">release</strong> branches, |
|
|
which are named <code class="code">release/X</code>, with <code class="code">X</code> being the release |
|
|
version number. |
|
|
</li></ol> |
|
|
|
|
|
<p>Note that we promise to our users that shared libraries from any FFmpeg |
|
|
release never break programs that have been <strong class="strong">compiled</strong> against |
|
|
previous versions of <strong class="strong">the same release series</strong> in any case! |
|
|
</p> |
|
|
<p>However, from time to time, we do make API changes that require adaptations |
|
|
in applications. Such changes are only allowed in (new) major releases and |
|
|
require further steps such as bumping library version numbers and/or |
|
|
adjustments to the symbol versioning file. Please discuss such changes |
|
|
on the <strong class="strong">ffmpeg-devel</strong> mailing list in time to allow forward planning. |
|
|
</p> |
|
|
<a class="anchor" id="Criteria-for-Point-Releases"></a><ul class="mini-toc"> |
|
|
<li><a href="#Criteria-for-Point-Releases-1" accesskey="1">Criteria for Point Releases</a></li> |
|
|
<li><a href="#Release-Checklist" accesskey="2">Release Checklist</a></li> |
|
|
</ul> |
|
|
<div class="section-level-extent" id="Criteria-for-Point-Releases-1"> |
|
|
<h3 class="section">9.1 Criteria for Point Releases</h3> |
|
|
|
|
|
<p>Changes that match the following criteria are valid candidates for |
|
|
inclusion into a point release: |
|
|
</p> |
|
|
<ol class="enumerate"> |
|
|
<li> Fixes a security issue, preferably identified by a <strong class="strong">CVE |
|
|
number</strong> issued by <a class="url" href="http://cve.mitre.org/">http://cve.mitre.org/</a>. |
|
|
|
|
|
</li><li> Fixes a documented bug in <a class="url" href="https://trac.ffmpeg.org">https://trac.ffmpeg.org</a>. |
|
|
|
|
|
</li><li> Improves the included documentation. |
|
|
|
|
|
</li><li> Retains both source code and binary compatibility with previous |
|
|
point releases of the same release branch. |
|
|
</li></ol> |
|
|
|
|
|
<p>The order for checking the rules is (1 OR 2 OR 3) AND 4. |
|
|
</p> |
|
|
|
|
|
</div> |
|
|
<div class="section-level-extent" id="Release-Checklist"> |
|
|
<h3 class="section">9.2 Release Checklist</h3> |
|
|
|
|
|
<p>The release process involves the following steps: |
|
|
</p> |
|
|
<ol class="enumerate"> |
|
|
<li> Ensure that the <samp class="file">RELEASE</samp> file contains the version number for |
|
|
the upcoming release. |
|
|
|
|
|
</li><li> Add the release at <a class="url" href="https://trac.ffmpeg.org/admin/ticket/versions">https://trac.ffmpeg.org/admin/ticket/versions</a>. |
|
|
|
|
|
</li><li> Announce the intent to do a release to the mailing list. |
|
|
|
|
|
</li><li> Make sure all relevant security fixes have been backported. See |
|
|
<a class="url" href="https://ffmpeg.org/security.html">https://ffmpeg.org/security.html</a>. |
|
|
|
|
|
</li><li> Ensure that the FATE regression suite still passes in the release |
|
|
branch on at least <strong class="strong">i386</strong> and <strong class="strong">amd64</strong> |
|
|
(cf. <a class="ref" href="#Regression-tests">Regression tests</a>). |
|
|
|
|
|
</li><li> Prepare the release tarballs in <code class="code">bz2</code> and <code class="code">gz</code> formats, and |
|
|
supplementing files that contain <code class="code">gpg</code> signatures |
|
|
|
|
|
</li><li> Publish the tarballs at <a class="url" href="https://ffmpeg.org/releases">https://ffmpeg.org/releases</a>. Create and |
|
|
push an annotated tag in the form <code class="code">nX</code>, with <code class="code">X</code> |
|
|
containing the version number. |
|
|
|
|
|
</li><li> Propose and send a patch to the <strong class="strong">ffmpeg-devel</strong> mailing list |
|
|
with a news entry for the website. |
|
|
|
|
|
</li><li> Publish the news entry. |
|
|
|
|
|
</li><li> Send an announcement to the mailing list. |
|
|
</li></ol> |
|
|
|
|
|
</div> |
|
|
</div> |
|
|
</div> |
|
|
|
|
|
|
|
|
|
|
|
</body> |
|
|
</html> |
|
|
|
