| 1 | \input texinfo @c -*- texinfo -*- |
| 2 | |
| 3 | @settitle Developer Documentation |
| 4 | @titlepage |
| 5 | @center @titlefont{Developer Documentation} |
| 6 | @end titlepage |
| 7 | |
| 8 | @top |
| 9 | |
| 10 | @contents |
| 11 | |
| 12 | @chapter Developers Guide |
| 13 | |
| 14 | @section Notes for external developers |
| 15 | |
| 16 | This document is mostly useful for internal FFmpeg developers. |
| 17 | External developers who need to use the API in their application should |
| 18 | refer to the API doxygen documentation in the public headers, and |
| 19 | check the examples in @file{doc/examples} and in the source code to |
| 20 | see how the public API is employed. |
| 21 | |
| 22 | You can use the FFmpeg libraries in your commercial program, but you |
| 23 | are encouraged to @emph{publish any patch you make}. In this case the |
| 24 | best way to proceed is to send your patches to the ffmpeg-devel |
| 25 | mailing list following the guidelines illustrated in the remainder of |
| 26 | this document. |
| 27 | |
| 28 | For more detailed legal information about the use of FFmpeg in |
| 29 | external programs read the @file{LICENSE} file in the source tree and |
| 30 | consult @url{http://ffmpeg.org/legal.html}. |
| 31 | |
| 32 | @section Contributing |
| 33 | |
| 34 | There are 3 ways by which code gets into ffmpeg. |
| 35 | @itemize @bullet |
| 36 | @item Submitting Patches to the main developer mailing list |
| 37 | see @ref{Submitting patches} for details. |
| 38 | @item Directly committing changes to the main tree. |
| 39 | @item Committing changes to a git clone, for example on github.com or |
| 40 | gitorious.org. And asking us to merge these changes. |
| 41 | @end itemize |
| 42 | |
| 43 | Whichever way, changes should be reviewed by the maintainer of the code |
| 44 | before they are committed. And they should follow the @ref{Coding Rules}. |
| 45 | The developer making the commit and the author are responsible for their changes |
| 46 | and should try to fix issues their commit causes. |
| 47 | |
| 48 | @anchor{Coding Rules} |
| 49 | @section Coding Rules |
| 50 | |
| 51 | @subsection Code formatting conventions |
| 52 | |
| 53 | There are the following guidelines regarding the indentation in files: |
| 54 | |
| 55 | @itemize @bullet |
| 56 | @item |
| 57 | Indent size is 4. |
| 58 | |
| 59 | @item |
| 60 | The TAB character is forbidden outside of Makefiles as is any |
| 61 | form of trailing whitespace. Commits containing either will be |
| 62 | rejected by the git repository. |
| 63 | |
| 64 | @item |
| 65 | You should try to limit your code lines to 80 characters; however, do so if |
| 66 | and only if this improves readability. |
| 67 | @end itemize |
| 68 | The presentation is one inspired by 'indent -i4 -kr -nut'. |
| 69 | |
| 70 | The main priority in FFmpeg is simplicity and small code size in order to |
| 71 | minimize the bug count. |
| 72 | |
| 73 | @subsection Comments |
| 74 | Use the JavaDoc/Doxygen format (see examples below) so that code documentation |
| 75 | can be generated automatically. All nontrivial functions should have a comment |
| 76 | above them explaining what the function does, even if it is just one sentence. |
| 77 | All structures and their member variables should be documented, too. |
| 78 | |
| 79 | Avoid Qt-style and similar Doxygen syntax with @code{!} in it, i.e. replace |
| 80 | @code{//!} with @code{///} and similar. Also @@ syntax should be employed |
| 81 | for markup commands, i.e. use @code{@@param} and not @code{\param}. |
| 82 | |
| 83 | @example |
| 84 | /** |
| 85 | * @@file |
| 86 | * MPEG codec. |
| 87 | * @@author ... |
| 88 | */ |
| 89 | |
| 90 | /** |
| 91 | * Summary sentence. |
| 92 | * more text ... |
| 93 | * ... |
| 94 | */ |
| 95 | typedef struct Foobar @{ |
| 96 | int var1; /**< var1 description */ |
| 97 | int var2; ///< var2 description |
| 98 | /** var3 description */ |
| 99 | int var3; |
| 100 | @} Foobar; |
| 101 | |
| 102 | /** |
| 103 | * Summary sentence. |
| 104 | * more text ... |
| 105 | * ... |
| 106 | * @@param my_parameter description of my_parameter |
| 107 | * @@return return value description |
| 108 | */ |
| 109 | int myfunc(int my_parameter) |
| 110 | ... |
| 111 | @end example |
| 112 | |
| 113 | @subsection C language features |
| 114 | |
| 115 | FFmpeg is programmed in the ISO C90 language with a few additional |
| 116 | features from ISO C99, namely: |
| 117 | |
| 118 | @itemize @bullet |
| 119 | @item |
| 120 | the @samp{inline} keyword; |
| 121 | |
| 122 | @item |
| 123 | @samp{//} comments; |
| 124 | |
| 125 | @item |
| 126 | designated struct initializers (@samp{struct s x = @{ .i = 17 @};}) |
| 127 | |
| 128 | @item |
| 129 | compound literals (@samp{x = (struct s) @{ 17, 23 @};}) |
| 130 | @end itemize |
| 131 | |
| 132 | These features are supported by all compilers we care about, so we will not |
| 133 | accept patches to remove their use unless they absolutely do not impair |
| 134 | clarity and performance. |
| 135 | |
| 136 | All code must compile with recent versions of GCC and a number of other |
| 137 | currently supported compilers. To ensure compatibility, please do not use |
| 138 | additional C99 features or GCC extensions. Especially watch out for: |
| 139 | |
| 140 | @itemize @bullet |
| 141 | @item |
| 142 | mixing statements and declarations; |
| 143 | |
| 144 | @item |
| 145 | @samp{long long} (use @samp{int64_t} instead); |
| 146 | |
| 147 | @item |
| 148 | @samp{__attribute__} not protected by @samp{#ifdef __GNUC__} or similar; |
| 149 | |
| 150 | @item |
| 151 | GCC statement expressions (@samp{(x = (@{ int y = 4; y; @})}). |
| 152 | @end itemize |
| 153 | |
| 154 | @subsection Naming conventions |
| 155 | All names should be composed with underscores (_), not CamelCase. For example, |
| 156 | @samp{avfilter_get_video_buffer} is an acceptable function name and |
| 157 | @samp{AVFilterGetVideo} is not. The exception from this are type names, like |
| 158 | for example structs and enums; they should always be in the CamelCase |
| 159 | |
| 160 | There are the following conventions for naming variables and functions: |
| 161 | |
| 162 | @itemize @bullet |
| 163 | @item |
| 164 | For local variables no prefix is required. |
| 165 | |
| 166 | @item |
| 167 | For file-scope variables and functions declared as @code{static}, no prefix |
| 168 | is required. |
| 169 | |
| 170 | @item |
| 171 | For variables and functions visible outside of file scope, but only used |
| 172 | internally by a library, an @code{ff_} prefix should be used, |
| 173 | e.g. @samp{ff_w64_demuxer}. |
| 174 | |
| 175 | @item |
| 176 | For variables and functions visible outside of file scope, used internally |
| 177 | across multiple libraries, use @code{avpriv_} as prefix, for example, |
| 178 | @samp{avpriv_aac_parse_header}. |
| 179 | |
| 180 | @item |
| 181 | Each library has its own prefix for public symbols, in addition to the |
| 182 | commonly used @code{av_} (@code{avformat_} for libavformat, |
| 183 | @code{avcodec_} for libavcodec, @code{swr_} for libswresample, etc). |
| 184 | Check the existing code and choose names accordingly. |
| 185 | Note that some symbols without these prefixes are also exported for |
| 186 | retro-compatibility reasons. These exceptions are declared in the |
| 187 | @code{lib<name>/lib<name>.v} files. |
| 188 | @end itemize |
| 189 | |
| 190 | Furthermore, name space reserved for the system should not be invaded. |
| 191 | Identifiers ending in @code{_t} are reserved by |
| 192 | @url{http://pubs.opengroup.org/onlinepubs/007904975/functions/xsh_chap02_02.html#tag_02_02_02, POSIX}. |
| 193 | Also avoid names starting with @code{__} or @code{_} followed by an uppercase |
| 194 | letter as they are reserved by the C standard. Names starting with @code{_} |
| 195 | are reserved at the file level and may not be used for externally visible |
| 196 | symbols. If in doubt, just avoid names starting with @code{_} altogether. |
| 197 | |
| 198 | @subsection Miscellaneous conventions |
| 199 | |
| 200 | @itemize @bullet |
| 201 | @item |
| 202 | fprintf and printf are forbidden in libavformat and libavcodec, |
| 203 | please use av_log() instead. |
| 204 | |
| 205 | @item |
| 206 | Casts should be used only when necessary. Unneeded parentheses |
| 207 | should also be avoided if they don't make the code easier to understand. |
| 208 | @end itemize |
| 209 | |
| 210 | @subsection Editor configuration |
| 211 | In order to configure Vim to follow FFmpeg formatting conventions, paste |
| 212 | the following snippet into your @file{.vimrc}: |
| 213 | @example |
| 214 | " indentation rules for FFmpeg: 4 spaces, no tabs |
| 215 | set expandtab |
| 216 | set shiftwidth=4 |
| 217 | set softtabstop=4 |
| 218 | set cindent |
| 219 | set cinoptions=(0 |
| 220 | " Allow tabs in Makefiles. |
| 221 | autocmd FileType make,automake set noexpandtab shiftwidth=8 softtabstop=8 |
| 222 | " Trailing whitespace and tabs are forbidden, so highlight them. |
| 223 | highlight ForbiddenWhitespace ctermbg=red guibg=red |
| 224 | match ForbiddenWhitespace /\s\+$\|\t/ |
| 225 | " Do not highlight spaces at the end of line while typing on that line. |
| 226 | autocmd InsertEnter * match ForbiddenWhitespace /\t\|\s\+\%#\@@<!$/ |
| 227 | @end example |
| 228 | |
| 229 | For Emacs, add these roughly equivalent lines to your @file{.emacs.d/init.el}: |
| 230 | @example |
| 231 | (c-add-style "ffmpeg" |
| 232 | '("k&r" |
| 233 | (c-basic-offset . 4) |
| 234 | (indent-tabs-mode . nil) |
| 235 | (show-trailing-whitespace . t) |
| 236 | (c-offsets-alist |
| 237 | (statement-cont . (c-lineup-assignments +))) |
| 238 | ) |
| 239 | ) |
| 240 | (setq c-default-style "ffmpeg") |
| 241 | @end example |
| 242 | |
| 243 | @section Development Policy |
| 244 | |
| 245 | @enumerate |
| 246 | @item |
| 247 | Contributions should be licensed under the |
| 248 | @uref{http://www.gnu.org/licenses/lgpl-2.1.html, LGPL 2.1}, |
| 249 | including an "or any later version" clause, or, if you prefer |
| 250 | a gift-style license, the |
| 251 | @uref{http://opensource.org/licenses/isc-license.txt, ISC} or |
| 252 | @uref{http://mit-license.org/, MIT} license. |
| 253 | @uref{http://www.gnu.org/licenses/gpl-2.0.html, GPL 2} including |
| 254 | an "or any later version" clause is also acceptable, but LGPL is |
| 255 | preferred. |
| 256 | If you add a new file, give it a proper license header. Do not copy and |
| 257 | paste it from a random place, use an existing file as template. |
| 258 | |
| 259 | @item |
| 260 | You must not commit code which breaks FFmpeg! (Meaning unfinished but |
| 261 | enabled code which breaks compilation or compiles but does not work or |
| 262 | breaks the regression tests) |
| 263 | You can commit unfinished stuff (for testing etc), but it must be disabled |
| 264 | (#ifdef etc) by default so it does not interfere with other developers' |
| 265 | work. |
| 266 | |
| 267 | @item |
| 268 | The commit message should have a short first line in the form of |
| 269 | a @samp{topic: short description} as a header, separated by a newline |
| 270 | from the body consisting of an explanation of why the change is necessary. |
| 271 | If the commit fixes a known bug on the bug tracker, the commit message |
| 272 | should include its bug ID. Referring to the issue on the bug tracker does |
| 273 | not exempt you from writing an excerpt of the bug in the commit message. |
| 274 | |
| 275 | @item |
| 276 | You do not have to over-test things. If it works for you, and you think it |
| 277 | should work for others, then commit. If your code has problems |
| 278 | (portability, triggers compiler bugs, unusual environment etc) they will be |
| 279 | reported and eventually fixed. |
| 280 | |
| 281 | @item |
| 282 | Do not commit unrelated changes together, split them into self-contained |
| 283 | pieces. Also do not forget that if part B depends on part A, but A does not |
| 284 | depend on B, then A can and should be committed first and separate from B. |
| 285 | Keeping changes well split into self-contained parts makes reviewing and |
| 286 | understanding them on the commit log mailing list easier. This also helps |
| 287 | in case of debugging later on. |
| 288 | Also if you have doubts about splitting or not splitting, do not hesitate to |
| 289 | ask/discuss it on the developer mailing list. |
| 290 | |
| 291 | @item |
| 292 | Do not change behavior of the programs (renaming options etc) or public |
| 293 | API or ABI without first discussing it on the ffmpeg-devel mailing list. |
| 294 | Do not remove functionality from the code. Just improve! |
| 295 | |
| 296 | Note: Redundant code can be removed. |
| 297 | |
| 298 | @item |
| 299 | Do not commit changes to the build system (Makefiles, configure script) |
| 300 | which change behavior, defaults etc, without asking first. The same |
| 301 | applies to compiler warning fixes, trivial looking fixes and to code |
| 302 | maintained by other developers. We usually have a reason for doing things |
| 303 | the way we do. Send your changes as patches to the ffmpeg-devel mailing |
| 304 | list, and if the code maintainers say OK, you may commit. This does not |
| 305 | apply to files you wrote and/or maintain. |
| 306 | |
| 307 | @item |
| 308 | We refuse source indentation and other cosmetic changes if they are mixed |
| 309 | with functional changes, such commits will be rejected and removed. Every |
| 310 | developer has his own indentation style, you should not change it. Of course |
| 311 | if you (re)write something, you can use your own style, even though we would |
| 312 | prefer if the indentation throughout FFmpeg was consistent (Many projects |
| 313 | force a given indentation style - we do not.). If you really need to make |
| 314 | indentation changes (try to avoid this), separate them strictly from real |
| 315 | changes. |
| 316 | |
| 317 | NOTE: If you had to put if()@{ .. @} over a large (> 5 lines) chunk of code, |
| 318 | then either do NOT change the indentation of the inner part within (do not |
| 319 | move it to the right)! or do so in a separate commit |
| 320 | |
| 321 | @item |
| 322 | Always fill out the commit log message. Describe in a few lines what you |
| 323 | changed and why. You can refer to mailing list postings if you fix a |
| 324 | particular bug. Comments such as "fixed!" or "Changed it." are unacceptable. |
| 325 | Recommended format: |
| 326 | |
| 327 | @example |
| 328 | area changed: Short 1 line description |
| 329 | |
| 330 | details describing what and why and giving references. |
| 331 | @end example |
| 332 | |
| 333 | @item |
| 334 | Make sure the author of the commit is set correctly. (see git commit --author) |
| 335 | If you apply a patch, send an |
| 336 | answer to ffmpeg-devel (or wherever you got the patch from) saying that |
| 337 | you applied the patch. |
| 338 | |
| 339 | @item |
| 340 | When applying patches that have been discussed (at length) on the mailing |
| 341 | list, reference the thread in the log message. |
| 342 | |
| 343 | @item |
| 344 | Do NOT commit to code actively maintained by others without permission. |
| 345 | Send a patch to ffmpeg-devel instead. If no one answers within a reasonable |
| 346 | timeframe (12h for build failures and security fixes, 3 days small changes, |
| 347 | 1 week for big patches) then commit your patch if you think it is OK. |
| 348 | Also note, the maintainer can simply ask for more time to review! |
| 349 | |
| 350 | @item |
| 351 | Subscribe to the ffmpeg-cvslog mailing list. The diffs of all commits |
| 352 | are sent there and reviewed by all the other developers. Bugs and possible |
| 353 | improvements or general questions regarding commits are discussed there. We |
| 354 | expect you to react if problems with your code are uncovered. |
| 355 | |
| 356 | @item |
| 357 | Update the documentation if you change behavior or add features. If you are |
| 358 | unsure how best to do this, send a patch to ffmpeg-devel, the documentation |
| 359 | maintainer(s) will review and commit your stuff. |
| 360 | |
| 361 | @item |
| 362 | Try to keep important discussions and requests (also) on the public |
| 363 | developer mailing list, so that all developers can benefit from them. |
| 364 | |
| 365 | @item |
| 366 | Never write to unallocated memory, never write over the end of arrays, |
| 367 | always check values read from some untrusted source before using them |
| 368 | as array index or other risky things. |
| 369 | |
| 370 | @item |
| 371 | Remember to check if you need to bump versions for the specific libav* |
| 372 | parts (libavutil, libavcodec, libavformat) you are changing. You need |
| 373 | to change the version integer. |
| 374 | Incrementing the first component means no backward compatibility to |
| 375 | previous versions (e.g. removal of a function from the public API). |
| 376 | Incrementing the second component means backward compatible change |
| 377 | (e.g. addition of a function to the public API or extension of an |
| 378 | existing data structure). |
| 379 | Incrementing the third component means a noteworthy binary compatible |
| 380 | change (e.g. encoder bug fix that matters for the decoder). The third |
| 381 | component always starts at 100 to distinguish FFmpeg from Libav. |
| 382 | |
| 383 | @item |
| 384 | Compiler warnings indicate potential bugs or code with bad style. If a type of |
| 385 | warning always points to correct and clean code, that warning should |
| 386 | be disabled, not the code changed. |
| 387 | Thus the remaining warnings can either be bugs or correct code. |
| 388 | If it is a bug, the bug has to be fixed. If it is not, the code should |
| 389 | be changed to not generate a warning unless that causes a slowdown |
| 390 | or obfuscates the code. |
| 391 | |
| 392 | @item |
| 393 | Make sure that no parts of the codebase that you maintain are missing from the |
| 394 | @file{MAINTAINERS} file. If something that you want to maintain is missing add it with |
| 395 | your name after it. |
| 396 | If at some point you no longer want to maintain some code, then please help |
| 397 | finding a new maintainer and also don't forget updating the @file{MAINTAINERS} file. |
| 398 | @end enumerate |
| 399 | |
| 400 | We think our rules are not too hard. If you have comments, contact us. |
| 401 | |
| 402 | @anchor{Submitting patches} |
| 403 | @section Submitting patches |
| 404 | |
| 405 | First, read the @ref{Coding Rules} above if you did not yet, in particular |
| 406 | the rules regarding patch submission. |
| 407 | |
| 408 | When you submit your patch, please use @code{git format-patch} or |
| 409 | @code{git send-email}. We cannot read other diffs :-) |
| 410 | |
| 411 | Also please do not submit a patch which contains several unrelated changes. |
| 412 | Split it into separate, self-contained pieces. This does not mean splitting |
| 413 | file by file. Instead, make the patch as small as possible while still |
| 414 | keeping it as a logical unit that contains an individual change, even |
| 415 | if it spans multiple files. This makes reviewing your patches much easier |
| 416 | for us and greatly increases your chances of getting your patch applied. |
| 417 | |
| 418 | Use the patcheck tool of FFmpeg to check your patch. |
| 419 | The tool is located in the tools directory. |
| 420 | |
| 421 | Run the @ref{Regression tests} before submitting a patch in order to verify |
| 422 | it does not cause unexpected problems. |
| 423 | |
| 424 | It also helps quite a bit if you tell us what the patch does (for example |
| 425 | 'replaces lrint by lrintf'), and why (for example '*BSD isn't C99 compliant |
| 426 | and has no lrint()') |
| 427 | |
| 428 | Also please if you send several patches, send each patch as a separate mail, |
| 429 | do not attach several unrelated patches to the same mail. |
| 430 | |
| 431 | Patches should be posted to the |
| 432 | @uref{http://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} |
| 433 | mailing list. Use @code{git send-email} when possible since it will properly |
| 434 | send patches without requiring extra care. If you cannot, then send patches |
| 435 | as base64-encoded attachments, so your patch is not trashed during |
| 436 | transmission. |
| 437 | |
| 438 | Your patch will be reviewed on the mailing list. You will likely be asked |
| 439 | to make some changes and are expected to send in an improved version that |
| 440 | incorporates the requests from the review. This process may go through |
| 441 | several iterations. Once your patch is deemed good enough, some developer |
| 442 | will pick it up and commit it to the official FFmpeg tree. |
| 443 | |
| 444 | Give us a few days to react. But if some time passes without reaction, |
| 445 | send a reminder by email. Your patch should eventually be dealt with. |
| 446 | |
| 447 | |
| 448 | @section New codecs or formats checklist |
| 449 | |
| 450 | @enumerate |
| 451 | @item |
| 452 | Did you use av_cold for codec initialization and close functions? |
| 453 | |
| 454 | @item |
| 455 | Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or |
| 456 | AVInputFormat/AVOutputFormat struct? |
| 457 | |
| 458 | @item |
| 459 | Did you bump the minor version number (and reset the micro version |
| 460 | number) in @file{libavcodec/version.h} or @file{libavformat/version.h}? |
| 461 | |
| 462 | @item |
| 463 | Did you register it in @file{allcodecs.c} or @file{allformats.c}? |
| 464 | |
| 465 | @item |
| 466 | Did you add the AVCodecID to @file{avcodec.h}? |
| 467 | When adding new codec IDs, also add an entry to the codec descriptor |
| 468 | list in @file{libavcodec/codec_desc.c}. |
| 469 | |
| 470 | @item |
| 471 | If it has a FourCC, did you add it to @file{libavformat/riff.c}, |
| 472 | even if it is only a decoder? |
| 473 | |
| 474 | @item |
| 475 | Did you add a rule to compile the appropriate files in the Makefile? |
| 476 | Remember to do this even if you're just adding a format to a file that is |
| 477 | already being compiled by some other rule, like a raw demuxer. |
| 478 | |
| 479 | @item |
| 480 | Did you add an entry to the table of supported formats or codecs in |
| 481 | @file{doc/general.texi}? |
| 482 | |
| 483 | @item |
| 484 | Did you add an entry in the Changelog? |
| 485 | |
| 486 | @item |
| 487 | If it depends on a parser or a library, did you add that dependency in |
| 488 | configure? |
| 489 | |
| 490 | @item |
| 491 | Did you @code{git add} the appropriate files before committing? |
| 492 | |
| 493 | @item |
| 494 | Did you make sure it compiles standalone, i.e. with |
| 495 | @code{configure --disable-everything --enable-decoder=foo} |
| 496 | (or @code{--enable-demuxer} or whatever your component is)? |
| 497 | @end enumerate |
| 498 | |
| 499 | |
| 500 | @section patch submission checklist |
| 501 | |
| 502 | @enumerate |
| 503 | @item |
| 504 | Does @code{make fate} pass with the patch applied? |
| 505 | |
| 506 | @item |
| 507 | Was the patch generated with git format-patch or send-email? |
| 508 | |
| 509 | @item |
| 510 | Did you sign off your patch? (git commit -s) |
| 511 | See @url{http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob_plain;f=Documentation/SubmittingPatches} for the meaning |
| 512 | of sign off. |
| 513 | |
| 514 | @item |
| 515 | Did you provide a clear git commit log message? |
| 516 | |
| 517 | @item |
| 518 | Is the patch against latest FFmpeg git master branch? |
| 519 | |
| 520 | @item |
| 521 | Are you subscribed to ffmpeg-devel? |
| 522 | (the list is subscribers only due to spam) |
| 523 | |
| 524 | @item |
| 525 | Have you checked that the changes are minimal, so that the same cannot be |
| 526 | achieved with a smaller patch and/or simpler final code? |
| 527 | |
| 528 | @item |
| 529 | If the change is to speed critical code, did you benchmark it? |
| 530 | |
| 531 | @item |
| 532 | If you did any benchmarks, did you provide them in the mail? |
| 533 | |
| 534 | @item |
| 535 | Have you checked that the patch does not introduce buffer overflows or |
| 536 | other security issues? |
| 537 | |
| 538 | @item |
| 539 | Did you test your decoder or demuxer against damaged data? If no, see |
| 540 | tools/trasher, the noise bitstream filter, and |
| 541 | @uref{http://caca.zoy.org/wiki/zzuf, zzuf}. Your decoder or demuxer |
| 542 | should not crash, end in a (near) infinite loop, or allocate ridiculous |
| 543 | amounts of memory when fed damaged data. |
| 544 | |
| 545 | @item |
| 546 | Does the patch not mix functional and cosmetic changes? |
| 547 | |
| 548 | @item |
| 549 | Did you add tabs or trailing whitespace to the code? Both are forbidden. |
| 550 | |
| 551 | @item |
| 552 | Is the patch attached to the email you send? |
| 553 | |
| 554 | @item |
| 555 | Is the mime type of the patch correct? It should be text/x-diff or |
| 556 | text/x-patch or at least text/plain and not application/octet-stream. |
| 557 | |
| 558 | @item |
| 559 | If the patch fixes a bug, did you provide a verbose analysis of the bug? |
| 560 | |
| 561 | @item |
| 562 | If the patch fixes a bug, did you provide enough information, including |
| 563 | a sample, so the bug can be reproduced and the fix can be verified? |
| 564 | Note please do not attach samples >100k to mails but rather provide a |
| 565 | URL, you can upload to ftp://upload.ffmpeg.org |
| 566 | |
| 567 | @item |
| 568 | Did you provide a verbose summary about what the patch does change? |
| 569 | |
| 570 | @item |
| 571 | Did you provide a verbose explanation why it changes things like it does? |
| 572 | |
| 573 | @item |
| 574 | Did you provide a verbose summary of the user visible advantages and |
| 575 | disadvantages if the patch is applied? |
| 576 | |
| 577 | @item |
| 578 | Did you provide an example so we can verify the new feature added by the |
| 579 | patch easily? |
| 580 | |
| 581 | @item |
| 582 | If you added a new file, did you insert a license header? It should be |
| 583 | taken from FFmpeg, not randomly copied and pasted from somewhere else. |
| 584 | |
| 585 | @item |
| 586 | You should maintain alphabetical order in alphabetically ordered lists as |
| 587 | long as doing so does not break API/ABI compatibility. |
| 588 | |
| 589 | @item |
| 590 | Lines with similar content should be aligned vertically when doing so |
| 591 | improves readability. |
| 592 | |
| 593 | @item |
| 594 | Consider to add a regression test for your code. |
| 595 | |
| 596 | @item |
| 597 | If you added YASM code please check that things still work with --disable-yasm |
| 598 | |
| 599 | @item |
| 600 | Make sure you check the return values of function and return appropriate |
| 601 | error codes. Especially memory allocation functions like @code{av_malloc()} |
| 602 | are notoriously left unchecked, which is a serious problem. |
| 603 | |
| 604 | @item |
| 605 | Test your code with valgrind and or Address Sanitizer to ensure it's free |
| 606 | of leaks, out of array accesses, etc. |
| 607 | @end enumerate |
| 608 | |
| 609 | @section Patch review process |
| 610 | |
| 611 | All patches posted to ffmpeg-devel will be reviewed, unless they contain a |
| 612 | clear note that the patch is not for the git master branch. |
| 613 | Reviews and comments will be posted as replies to the patch on the |
| 614 | mailing list. The patch submitter then has to take care of every comment, |
| 615 | that can be by resubmitting a changed patch or by discussion. Resubmitted |
| 616 | patches will themselves be reviewed like any other patch. If at some point |
| 617 | a patch passes review with no comments then it is approved, that can for |
| 618 | simple and small patches happen immediately while large patches will generally |
| 619 | have to be changed and reviewed many times before they are approved. |
| 620 | After a patch is approved it will be committed to the repository. |
| 621 | |
| 622 | We will review all submitted patches, but sometimes we are quite busy so |
| 623 | especially for large patches this can take several weeks. |
| 624 | |
| 625 | If you feel that the review process is too slow and you are willing to try to |
| 626 | take over maintainership of the area of code you change then just clone |
| 627 | git master and maintain the area of code there. We will merge each area from |
| 628 | where its best maintained. |
| 629 | |
| 630 | When resubmitting patches, please do not make any significant changes |
| 631 | not related to the comments received during review. Such patches will |
| 632 | be rejected. Instead, submit significant changes or new features as |
| 633 | separate patches. |
| 634 | |
| 635 | @anchor{Regression tests} |
| 636 | @section Regression tests |
| 637 | |
| 638 | Before submitting a patch (or committing to the repository), you should at least |
| 639 | test that you did not break anything. |
| 640 | |
| 641 | Running 'make fate' accomplishes this, please see @url{fate.html} for details. |
| 642 | |
| 643 | [Of course, some patches may change the results of the regression tests. In |
| 644 | this case, the reference results of the regression tests shall be modified |
| 645 | accordingly]. |
| 646 | |
| 647 | @subsection Adding files to the fate-suite dataset |
| 648 | |
| 649 | When there is no muxer or encoder available to generate test media for a |
| 650 | specific test then the media has to be inlcuded in the fate-suite. |
| 651 | First please make sure that the sample file is as small as possible to test the |
| 652 | respective decoder or demuxer sufficiently. Large files increase network |
| 653 | bandwidth and disk space requirements. |
| 654 | Once you have a working fate test and fate sample, provide in the commit |
| 655 | message or introductionary message for the patch series that you post to |
| 656 | the ffmpeg-devel mailing list, a direct link to download the sample media. |
| 657 | |
| 658 | |
| 659 | @subsection Visualizing Test Coverage |
| 660 | |
| 661 | The FFmpeg build system allows visualizing the test coverage in an easy |
| 662 | manner with the coverage tools @code{gcov}/@code{lcov}. This involves |
| 663 | the following steps: |
| 664 | |
| 665 | @enumerate |
| 666 | @item |
| 667 | Configure to compile with instrumentation enabled: |
| 668 | @code{configure --toolchain=gcov}. |
| 669 | |
| 670 | @item |
| 671 | Run your test case, either manually or via FATE. This can be either |
| 672 | the full FATE regression suite, or any arbitrary invocation of any |
| 673 | front-end tool provided by FFmpeg, in any combination. |
| 674 | |
| 675 | @item |
| 676 | Run @code{make lcov} to generate coverage data in HTML format. |
| 677 | |
| 678 | @item |
| 679 | View @code{lcov/index.html} in your preferred HTML viewer. |
| 680 | @end enumerate |
| 681 | |
| 682 | You can use the command @code{make lcov-reset} to reset the coverage |
| 683 | measurements. You will need to rerun @code{make lcov} after running a |
| 684 | new test. |
| 685 | |
| 686 | @subsection Using Valgrind |
| 687 | |
| 688 | The configure script provides a shortcut for using valgrind to spot bugs |
| 689 | related to memory handling. Just add the option |
| 690 | @code{--toolchain=valgrind-memcheck} or @code{--toolchain=valgrind-massif} |
| 691 | to your configure line, and reasonable defaults will be set for running |
| 692 | FATE under the supervision of either the @strong{memcheck} or the |
| 693 | @strong{massif} tool of the valgrind suite. |
| 694 | |
| 695 | In case you need finer control over how valgrind is invoked, use the |
| 696 | @code{--target-exec='valgrind <your_custom_valgrind_options>} option in |
| 697 | your configure line instead. |
| 698 | |
| 699 | @anchor{Release process} |
| 700 | @section Release process |
| 701 | |
| 702 | FFmpeg maintains a set of @strong{release branches}, which are the |
| 703 | recommended deliverable for system integrators and distributors (such as |
| 704 | Linux distributions, etc.). At regular times, a @strong{release |
| 705 | manager} prepares, tests and publishes tarballs on the |
| 706 | @url{http://ffmpeg.org} website. |
| 707 | |
| 708 | There are two kinds of releases: |
| 709 | |
| 710 | @enumerate |
| 711 | @item |
| 712 | @strong{Major releases} always include the latest and greatest |
| 713 | features and functionality. |
| 714 | |
| 715 | @item |
| 716 | @strong{Point releases} are cut from @strong{release} branches, |
| 717 | which are named @code{release/X}, with @code{X} being the release |
| 718 | version number. |
| 719 | @end enumerate |
| 720 | |
| 721 | Note that we promise to our users that shared libraries from any FFmpeg |
| 722 | release never break programs that have been @strong{compiled} against |
| 723 | previous versions of @strong{the same release series} in any case! |
| 724 | |
| 725 | However, from time to time, we do make API changes that require adaptations |
| 726 | in applications. Such changes are only allowed in (new) major releases and |
| 727 | require further steps such as bumping library version numbers and/or |
| 728 | adjustments to the symbol versioning file. Please discuss such changes |
| 729 | on the @strong{ffmpeg-devel} mailing list in time to allow forward planning. |
| 730 | |
| 731 | @anchor{Criteria for Point Releases} |
| 732 | @subsection Criteria for Point Releases |
| 733 | |
| 734 | Changes that match the following criteria are valid candidates for |
| 735 | inclusion into a point release: |
| 736 | |
| 737 | @enumerate |
| 738 | @item |
| 739 | Fixes a security issue, preferably identified by a @strong{CVE |
| 740 | number} issued by @url{http://cve.mitre.org/}. |
| 741 | |
| 742 | @item |
| 743 | Fixes a documented bug in @url{https://trac.ffmpeg.org}. |
| 744 | |
| 745 | @item |
| 746 | Improves the included documentation. |
| 747 | |
| 748 | @item |
| 749 | Retains both source code and binary compatibility with previous |
| 750 | point releases of the same release branch. |
| 751 | @end enumerate |
| 752 | |
| 753 | The order for checking the rules is (1 OR 2 OR 3) AND 4. |
| 754 | |
| 755 | |
| 756 | @subsection Release Checklist |
| 757 | |
| 758 | The release process involves the following steps: |
| 759 | |
| 760 | @enumerate |
| 761 | @item |
| 762 | Ensure that the @file{RELEASE} file contains the version number for |
| 763 | the upcoming release. |
| 764 | |
| 765 | @item |
| 766 | Add the release at @url{https://trac.ffmpeg.org/admin/ticket/versions}. |
| 767 | |
| 768 | @item |
| 769 | Announce the intent to do a release to the mailing list. |
| 770 | |
| 771 | @item |
| 772 | Make sure all relevant security fixes have been backported. See |
| 773 | @url{https://ffmpeg.org/security.html}. |
| 774 | |
| 775 | @item |
| 776 | Ensure that the FATE regression suite still passes in the release |
| 777 | branch on at least @strong{i386} and @strong{amd64} |
| 778 | (cf. @ref{Regression tests}). |
| 779 | |
| 780 | @item |
| 781 | Prepare the release tarballs in @code{bz2} and @code{gz} formats, and |
| 782 | supplementing files that contain @code{gpg} signatures |
| 783 | |
| 784 | @item |
| 785 | Publish the tarballs at @url{http://ffmpeg.org/releases}. Create and |
| 786 | push an annotated tag in the form @code{nX}, with @code{X} |
| 787 | containing the version number. |
| 788 | |
| 789 | @item |
| 790 | Propose and send a patch to the @strong{ffmpeg-devel} mailing list |
| 791 | with a news entry for the website. |
| 792 | |
| 793 | @item |
| 794 | Publish the news entry. |
| 795 | |
| 796 | @item |
| 797 | Send announcement to the mailing list. |
| 798 | @end enumerate |
| 799 | |
| 800 | @bye |