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