Commit | Line | Data |
---|---|---|
2ba45a60 DM |
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 |