mirror of https://git.ffmpeg.org/ffmpeg.git
doc: update developer guide
Refer to the current submission policy and refer to FATE in the Regression Test section.
This commit is contained in:
parent
86a89ae3f2
commit
4c362a5212
|
@ -23,16 +23,22 @@ audio or video streams.
|
|||
|
||||
@end itemize
|
||||
|
||||
@section Integrating libavcodec or libavformat in your program
|
||||
@section Integrating libav in your program
|
||||
|
||||
You can integrate all the source code of the libraries to link them
|
||||
statically to avoid any version problem. All you need is to provide a
|
||||
'config.mak' and a 'config.h' in the parent directory. See the defines
|
||||
generated by ./configure to understand what is needed.
|
||||
Shared libraries should be used whenever is possible in order to reduce
|
||||
the effort distributors have to pour to support programs and to ensure
|
||||
only the public api is used.
|
||||
|
||||
You can use libavcodec or libavformat in your commercial program, but
|
||||
@emph{any patch you make must be published}. The best way to proceed is
|
||||
to send your patches to the Libav mailing list.
|
||||
You can use Libav in your commercial program, but you must abide to the
|
||||
license, LGPL or GPL depending on the specific features used, please refer
|
||||
to @url{http://libav.org/legal.html} for a quick checklist and to
|
||||
@url{http://git.libav.org/?p=libav.git;a=blob;f=COPYING.GPLv2},
|
||||
@url{http://git.libav.org/?p=libav.git;a=blob;f=COPYING.GPLv3},
|
||||
@url{http://git.libav.org/?p=libav.git;a=blob;f=COPYING.LGPLv2.1},
|
||||
@url{http://git.libav.org/?p=libav.git;a=blob;f=COPYING.LGPLv3} for the
|
||||
exact text of the licenses.
|
||||
Any modification to the source code can be suggested for inclusion.
|
||||
The best way to proceed is to send your patches to the Libav mailing list.
|
||||
|
||||
@anchor{Coding Rules}
|
||||
@section Coding Rules
|
||||
|
@ -129,17 +135,33 @@ should also be avoided if they don't make the code easier to understand.
|
|||
an "or any later version" clause is also acceptable, but LGPL is
|
||||
preferred.
|
||||
@item
|
||||
You must not commit code which breaks Libav! (Meaning unfinished but
|
||||
enabled code which breaks compilation or compiles but does not work or
|
||||
breaks the regression tests)
|
||||
You can commit unfinished stuff (for testing etc), but it must be disabled
|
||||
(#ifdef etc) by default so it does not interfere with other developers'
|
||||
work.
|
||||
All the patches MUST be reviewed in the mailing list before they are
|
||||
committed.
|
||||
@item
|
||||
You do not have to over-test things. If it works for you, and you think it
|
||||
should work for others, then commit. If your code has problems
|
||||
(portability, triggers compiler bugs, unusual environment etc) they will be
|
||||
reported and eventually fixed.
|
||||
The Libav coding style should remain consistent. Changes to
|
||||
conform will be suggested during the review or implemented on commit.
|
||||
@item
|
||||
Patches should be generated using @code{git format-patch} or directly sent
|
||||
using @code{git send-email}.
|
||||
Please make sure you give the proper credit by setting the correct author
|
||||
in the commit.
|
||||
@item
|
||||
The commit message should have a short first line in the form of
|
||||
@samp{topic: short description} as header, separated by a newline
|
||||
from the body consting in few lines explaining the reason of the patch.
|
||||
Referring to the issue on the bug tracker does not exempt to report an
|
||||
excerpt of the bug.
|
||||
@item
|
||||
Work in progress patches should be sent to the mailing list with the [WIP]
|
||||
or the [RFC] tag.
|
||||
@item
|
||||
Branches in public personal repos are advised as way to
|
||||
work on issues collaboratively.
|
||||
@item
|
||||
You do not have to over-test things. If it works for you and you think it
|
||||
should work for others, send it to the mailing list for review.
|
||||
If you have doubt about portability please state it in the submission so
|
||||
people with specific hardware could test it.
|
||||
@item
|
||||
Do not commit unrelated changes together, split them into self-contained
|
||||
pieces. Also do not forget that if part B depends on part A, but A does not
|
||||
|
@ -147,70 +169,32 @@ should also be avoided if they don't make the code easier to understand.
|
|||
Keeping changes well split into self-contained parts makes reviewing and
|
||||
understanding them on the commit log mailing list easier. This also helps
|
||||
in case of debugging later on.
|
||||
Also if you have doubts about splitting or not splitting, do not hesitate to
|
||||
ask/discuss it on the developer mailing list.
|
||||
@item
|
||||
Do not change behavior of the programs (renaming options etc) or public
|
||||
API or ABI without first discussing it on the libav-devel mailing list.
|
||||
Do not remove functionality from the code. Just improve!
|
||||
|
||||
Note: Redundant code can be removed.
|
||||
@item
|
||||
Do not commit changes to the build system (Makefiles, configure script)
|
||||
which change behavior, defaults etc, without asking first. The same
|
||||
applies to compiler warning fixes, trivial looking fixes and to code
|
||||
maintained by other developers. We usually have a reason for doing things
|
||||
the way we do. Send your changes as patches to the libav-devel mailing
|
||||
list, and if the code maintainers say OK, you may commit. This does not
|
||||
apply to files you wrote and/or maintain.
|
||||
@item
|
||||
We refuse source indentation and other cosmetic changes if they are mixed
|
||||
with functional changes, such commits will be rejected and removed. Every
|
||||
developer has his own indentation style, you should not change it. Of course
|
||||
if you (re)write something, you can use your own style, even though we would
|
||||
prefer if the indentation throughout Libav was consistent (Many projects
|
||||
force a given indentation style - we do not.). If you really need to make
|
||||
indentation changes (try to avoid this), separate them strictly from real
|
||||
changes.
|
||||
|
||||
NOTE: If you had to put if()@{ .. @} over a large (> 5 lines) chunk of code,
|
||||
then either do NOT change the indentation of the inner part within (do not
|
||||
move it to the right)! or do so in a separate commit
|
||||
@item
|
||||
Always fill out the commit log message. Describe in a few lines what you
|
||||
changed and why. You can refer to mailing list postings if you fix a
|
||||
particular bug. Comments such as "fixed!" or "Changed it." are unacceptable.
|
||||
@item
|
||||
If you apply a patch by someone else, include the name and email address in
|
||||
the log message. Since the libav-commits mailing list is publicly
|
||||
archived you should add some SPAM protection to the email address. Send an
|
||||
answer to libav-devel (or wherever you got the patch from) saying that
|
||||
you applied the patch.
|
||||
Patches that change behavior of the programs (renaming options etc) or
|
||||
public API or ABI should be discussed in depth and possible few days should
|
||||
pass between discussion and commit.
|
||||
Changes to the build system (Makefiles, configure script) which alter
|
||||
the expected behavior should be considered in the same regard.
|
||||
@item
|
||||
When applying patches that have been discussed (at length) on the mailing
|
||||
list, reference the thread in the log message.
|
||||
@item
|
||||
Do NOT commit to code actively maintained by others without permission.
|
||||
Send a patch to libav-devel instead. If no one answers within a reasonable
|
||||
timeframe (12h for build failures and security fixes, 3 days small changes,
|
||||
1 week for big patches) then commit your patch if you think it is OK.
|
||||
Also note, the maintainer can simply ask for more time to review!
|
||||
@item
|
||||
Subscribe to the libav-commits mailing list. The diffs of all commits
|
||||
are sent there and reviewed by all the other developers. Bugs and possible
|
||||
improvements or general questions regarding commits are discussed there. We
|
||||
expect you to react if problems with your code are uncovered.
|
||||
Subscribe to the libav-devel and libav-commits mailing list.
|
||||
Bugs and possible improvements or general questions regarding commits
|
||||
are discussed on libav-devel. We expect you to react if problems with
|
||||
your code are uncovered.
|
||||
@item
|
||||
Update the documentation if you change behavior or add features. If you are
|
||||
unsure how best to do this, send a patch to libav-devel, the documentation
|
||||
maintainer(s) will review and commit your stuff.
|
||||
unsure how best to do this, send an [RFC] patch to libav-devel.
|
||||
@item
|
||||
Try to keep important discussions and requests (also) on the public
|
||||
developer mailing list, so that all developers can benefit from them.
|
||||
All discussions and decisions should be reported on the public developer
|
||||
mailing list, so that there is a reference to them.
|
||||
Other media (e.g. IRC) should be used for coordination and immediate
|
||||
collaboration.
|
||||
@item
|
||||
Never write to unallocated memory, never write over the end of arrays,
|
||||
always check values read from some untrusted source before using them
|
||||
as array index or other risky things.
|
||||
as array index or other risky things. Always use valgrind to doublecheck.
|
||||
@item
|
||||
Remember to check if you need to bump versions for the specific libav
|
||||
parts (libavutil, libavcodec, libavformat) you are changing. You need
|
||||
|
@ -223,13 +207,12 @@ should also be avoided if they don't make the code easier to understand.
|
|||
Incrementing the third component means a noteworthy binary compatible
|
||||
change (e.g. encoder bug fix that matters for the decoder).
|
||||
@item
|
||||
Compiler warnings indicate potential bugs or code with bad style. If a type of
|
||||
warning always points to correct and clean code, that warning should
|
||||
be disabled, not the code changed.
|
||||
Thus the remaining warnings can either be bugs or correct code.
|
||||
Compiler warnings indicate potential bugs or code with bad style.
|
||||
If it is a bug, the bug has to be fixed. If it is not, the code should
|
||||
be changed to not generate a warning unless that causes a slowdown
|
||||
or obfuscates the code.
|
||||
If a type of warning leads to too many false positives, that warning
|
||||
should be disabled, not the code changed.
|
||||
@item
|
||||
If you add a new file, give it a proper license header. Do not copy and
|
||||
paste it from a random place, use an existing file as template.
|
||||
|
@ -237,16 +220,15 @@ should also be avoided if they don't make the code easier to understand.
|
|||
|
||||
We think our rules are not too hard. If you have comments, contact us.
|
||||
|
||||
Note, these rules are mostly borrowed from the MPlayer project.
|
||||
Note, some rules were borrowed from the MPlayer project.
|
||||
|
||||
@section Submitting patches
|
||||
|
||||
First, (@pxref{Coding Rules}) above if you did not yet.
|
||||
First, read the (@pxref{Coding Rules}) above if you did not yet, in particular
|
||||
the rules regarding patch submission.
|
||||
|
||||
When you submit your patch, try to send a unified diff (diff '-up'
|
||||
option). We cannot read other diffs :-)
|
||||
|
||||
Also please do not submit a patch which contains several unrelated changes.
|
||||
As stated already, please do not submit a patch which contains several
|
||||
unrelated changes.
|
||||
Split it into separate, self-contained pieces. This does not mean splitting
|
||||
file by file. Instead, make the patch as small as possible while still
|
||||
keeping it as a logical unit that contains an individual change, even
|
||||
|
@ -256,8 +238,8 @@ for us and greatly increases your chances of getting your patch applied.
|
|||
Use the patcheck tool of Libav to check your patch.
|
||||
The tool is located in the tools directory.
|
||||
|
||||
Run the regression tests before submitting a patch so that you can
|
||||
verify that there are no big problems.
|
||||
Run the @pxref{Regression Tests} before submitting a patch in order to verify
|
||||
it does not cause unexpected problems.
|
||||
|
||||
Patches should be posted as base64 encoded attachments (or any other
|
||||
encoding which ensures that the patch will not be trashed during
|
||||
|
@ -266,16 +248,20 @@ transmission) to the libav-devel mailing list, see
|
|||
|
||||
It also helps quite a bit if you tell us what the patch does (for example
|
||||
'replaces lrint by lrintf'), and why (for example '*BSD isn't C99 compliant
|
||||
and has no lrint()')
|
||||
and has no lrint()'). This kind of explanation should be the body of the
|
||||
commit message.
|
||||
|
||||
Also please if you send several patches, send each patch as a separate mail,
|
||||
do not attach several unrelated patches to the same mail.
|
||||
|
||||
Use @code{git send-email} when possible since it will properly send patches
|
||||
without requiring extra care.
|
||||
|
||||
Your patch will be reviewed on the mailing list. You will likely be asked
|
||||
to make some changes and are expected to send in an improved version that
|
||||
incorporates the requests from the review. This process may go through
|
||||
several iterations. Once your patch is deemed good enough, some developer
|
||||
will pick it up and commit it to the official Libav tree.
|
||||
several iterations. Once your patch is deemed good enough, it will be
|
||||
committed to the official Libav tree.
|
||||
|
||||
Give us a few days to react. But if some time passes without reaction,
|
||||
send a reminder by email. Your patch should eventually be dealt with.
|
||||
|
@ -301,8 +287,8 @@ send a reminder by email. Your patch should eventually be dealt with.
|
|||
even if it is only a decoder?
|
||||
@item
|
||||
Did you add a rule to compile the appropriate files in the Makefile?
|
||||
Remember to do this even if you're just adding a format to a file that is
|
||||
already being compiled by some other rule, like a raw demuxer.
|
||||
Remember to do this even if you are just adding a format to a file that
|
||||
is already being compiled by some other rule, like a raw demuxer.
|
||||
@item
|
||||
Did you add an entry to the table of supported formats or codecs in
|
||||
@file{doc/general.texi}?
|
||||
|
@ -312,7 +298,7 @@ send a reminder by email. Your patch should eventually be dealt with.
|
|||
If it depends on a parser or a library, did you add that dependency in
|
||||
configure?
|
||||
@item
|
||||
Did you "git add" the appropriate files before committing?
|
||||
Did you @code{git add} the appropriate files before committing?
|
||||
@end enumerate
|
||||
|
||||
@section patch submission checklist
|
||||
|
@ -322,13 +308,12 @@ send a reminder by email. Your patch should eventually be dealt with.
|
|||
Do the regression tests pass with the patch applied?
|
||||
@item
|
||||
Does @code{make checkheaders} pass with the patch applied?
|
||||
@item
|
||||
Is the patch a unified diff?
|
||||
@item
|
||||
Is the patch against latest Libav git master branch?
|
||||
@item
|
||||
Are you subscribed to ffmpeg-dev?
|
||||
(the list is subscribers only due to spam)
|
||||
Are you subscribed to libav-devel?
|
||||
(@url{https://lists.libav.org/mailman/listinfo/libav-devel}
|
||||
the list is subscribers)
|
||||
@item
|
||||
Have you checked that the changes are minimal, so that the same cannot be
|
||||
achieved with a smaller patch and/or simpler final code?
|
||||
|
@ -343,9 +328,6 @@ send a reminder by email. Your patch should eventually be dealt with.
|
|||
Did you test your decoder or demuxer against damaged data? If no, see
|
||||
tools/trasher and the noise bitstream filter. Your decoder or demuxer
|
||||
should not crash or end in a (near) infinite loop when fed damaged data.
|
||||
@item
|
||||
Is the patch created from the root of the source tree, so it can be
|
||||
applied with @code{patch -p0}?
|
||||
@item
|
||||
Does the patch not mix functional and cosmetic changes?
|
||||
@item
|
||||
|
@ -381,8 +363,6 @@ send a reminder by email. Your patch should eventually be dealt with.
|
|||
@item
|
||||
Lines with similar content should be aligned vertically when doing so
|
||||
improves readability.
|
||||
@item
|
||||
Did you provide a suggestion for a clear commit log message?
|
||||
@end enumerate
|
||||
|
||||
@section Patch review process
|
||||
|
@ -401,37 +381,22 @@ After a patch is approved it will be committed to the repository.
|
|||
We will review all submitted patches, but sometimes we are quite busy so
|
||||
especially for large patches this can take several weeks.
|
||||
|
||||
When resubmitting patches, please do not make any significant changes
|
||||
not related to the comments received during review. Such patches will
|
||||
be rejected. Instead, submit significant changes or new features as
|
||||
separate patches.
|
||||
When resubmitting patches, if their size grew or during the review different
|
||||
issues arisen please split the patch so each issue has a specific patch.
|
||||
|
||||
@section Regression tests
|
||||
@anchor{Regression Tests}
|
||||
@section Regression Tests
|
||||
|
||||
Before submitting a patch (or committing to the repository), you should at least
|
||||
test that you did not break anything.
|
||||
Before submitting a patch (or committing to the repository), you should at
|
||||
least make sure that it does not break anything.
|
||||
|
||||
The regression tests build a synthetic video stream and a synthetic
|
||||
audio stream. These are then encoded and decoded with all codecs or
|
||||
formats. The CRC (or MD5) of each generated file is recorded in a
|
||||
result file. A 'diff' is launched to compare the reference results and
|
||||
the result file. The output is checked immediately after each test
|
||||
has run.
|
||||
If the code changed has already a test present in FATE you should run it,
|
||||
otherwise it is advised to add it.
|
||||
|
||||
The regression tests then go on to test the FFserver code with a
|
||||
limited set of streams. It is important that this step runs correctly
|
||||
as well.
|
||||
Improvements to codec or demuxer might change the FATE results. Make sure
|
||||
to commit the update reference with the change and to explain in the comment
|
||||
why the expected result changed.
|
||||
|
||||
Run 'make test' to test all the codecs and formats. Commands like
|
||||
'make regtest-mpeg2' can be used to run a single test. By default,
|
||||
make will abort if any test fails. To run all tests regardless,
|
||||
use make -k. To get a more verbose output, use 'make V=1 test' or
|
||||
'make V=2 test'.
|
||||
|
||||
Run 'make fulltest' to test all the codecs, formats and FFserver.
|
||||
|
||||
[Of course, some patches may change the results of the regression tests. In
|
||||
this case, the reference results of the regression tests shall be modified
|
||||
accordingly].
|
||||
Please refer to @file{doc/fate.txt}.
|
||||
|
||||
@bye
|
||||
|
|
Loading…
Reference in New Issue