forked from FFmpeg/FFmpeg
-
Notifications
You must be signed in to change notification settings - Fork 4
doc/developer: Restructure docs about patch submission #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
softworkz
wants to merge
4
commits into
ffstaging:master
Choose a base branch
from
softworkz:submit_website_update
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7923010
doc/developer: Move checklist into Submitting Patches chapter
softworkz ae47e8b
doc/developer: Move codecs/formats checklist into Development Policy …
softworkz 866fd3a
doc/developer: Reorder Submission procedures content
softworkz 2e4a47f
doc/developer: Merge Review paragraphs and deduplicate
softworkz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -651,6 +651,59 @@ Removing previously deprecated APIs. | |
| Performing ABI- but not API-breaking changes, like reordering struct contents. | ||
| @end itemize | ||
|
|
||
|
|
||
| @section New codecs or formats checklist | ||
|
|
||
| @enumerate | ||
| @item | ||
| Did you use av_cold for codec initialization and close functions? | ||
|
|
||
| @item | ||
| Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or | ||
| AVInputFormat/AVOutputFormat struct? | ||
|
|
||
| @item | ||
| Did you bump the minor version number (and reset the micro version | ||
| number) in @file{libavcodec/version.h} or @file{libavformat/version.h}? | ||
|
|
||
| @item | ||
| Did you register it in @file{allcodecs.c} or @file{allformats.c}? | ||
|
|
||
| @item | ||
| Did you add the AVCodecID to @file{codec_id.h}? | ||
| When adding new codec IDs, also add an entry to the codec descriptor | ||
| list in @file{libavcodec/codec_desc.c}. | ||
|
|
||
| @item | ||
| If it has a FourCC, did you add it to @file{libavformat/riff.c}, | ||
| 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. | ||
|
|
||
| @item | ||
| Did you add an entry to the table of supported formats or codecs in | ||
| @file{doc/general_contents.texi}? | ||
|
|
||
| @item | ||
| Did you add an entry in the Changelog? | ||
|
|
||
| @item | ||
| If it depends on a parser or a library, did you add that dependency in | ||
| configure? | ||
|
|
||
| @item | ||
| Did you @code{git add} the appropriate files before committing? | ||
|
|
||
| @item | ||
| Did you make sure it compiles standalone, i.e. with | ||
| @code{configure --disable-everything --enable-decoder=foo} | ||
| (or @code{--enable-demuxer} or whatever your component is)? | ||
| @end enumerate | ||
|
|
||
|
|
||
| @section Documentation/Other | ||
| @subheading Subscribe to the ffmpeg-devel mailing list. | ||
| It is important to be subscribed to the | ||
|
|
@@ -699,10 +752,7 @@ We think our rules are not too hard. If you have comments, contact us. | |
| First, read the @ref{Coding Rules} above if you did not yet, in particular | ||
| the rules regarding patch submission. | ||
|
|
||
| When you submit your patch, please use @code{git format-patch} or | ||
| @code{git send-email}. We cannot read other diffs :-). | ||
|
|
||
| Also please do not submit a patch which contains several unrelated changes. | ||
| 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 | ||
|
|
@@ -715,108 +765,8 @@ The tool is located in the tools directory. | |
| Run the @ref{Regression tests} before submitting a patch in order to verify | ||
| it does not cause unexpected problems. | ||
|
|
||
| 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()') | ||
|
|
||
| Also please if you send several patches, send each patch as a separate mail, | ||
| do not attach several unrelated patches to the same mail. | ||
|
|
||
| Patches should be posted to the | ||
| @uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} | ||
| mailing list. Use @code{git send-email} when possible since it will properly | ||
| send patches without requiring extra care. If you cannot, then send patches | ||
| as base64-encoded attachments, so your patch is not trashed during | ||
| transmission. Also ensure the correct mime type is used | ||
| (text/x-diff or text/x-patch or at least text/plain) and that only one | ||
| patch is inline or attached per mail. | ||
| You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show up, its mime type | ||
| likely was wrong. | ||
|
|
||
| @subheading How to setup git send-email? | ||
|
|
||
| Please see @url{https://git-send-email.io/}. | ||
| For gmail additionally see @url{https://shallowsky.com/blog/tech/email/gmail-app-passwds.html}. | ||
|
|
||
| @subheading Sending patches from email clients | ||
| Using @code{git send-email} might not be desirable for everyone. The | ||
| following trick allows to send patches via email clients in a safe | ||
| way. It has been tested with Outlook and Thunderbird (with X-Unsent | ||
| extension) and might work with other applications. | ||
|
|
||
| Create your patch like this: | ||
|
|
||
| @verbatim | ||
| git format-patch -s -o "outputfolder" --add-header "X-Unsent: 1" --suffix .eml --to [email protected] -1 1a2b3c4d | ||
| @end verbatim | ||
|
|
||
| Now you'll just need to open the eml file with the email application | ||
| and execute 'Send'. | ||
|
|
||
| @subheading Reviews | ||
| Your patch will be reviewed on the mailing list. You will likely be asked | ||
| to make some changes and are expected to send in an improved version that | ||
| incorporates the requests from the review. This process may go through | ||
| several iterations. Once your patch is deemed good enough, some developer | ||
| will pick it up and commit it to the official FFmpeg tree. | ||
|
|
||
| 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. | ||
|
|
||
|
|
||
| @chapter New codecs or formats checklist | ||
|
|
||
| @enumerate | ||
| @item | ||
| Did you use av_cold for codec initialization and close functions? | ||
|
|
||
| @item | ||
| Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or | ||
| AVInputFormat/AVOutputFormat struct? | ||
|
|
||
| @item | ||
| Did you bump the minor version number (and reset the micro version | ||
| number) in @file{libavcodec/version.h} or @file{libavformat/version.h}? | ||
|
|
||
| @item | ||
| Did you register it in @file{allcodecs.c} or @file{allformats.c}? | ||
|
|
||
| @item | ||
| Did you add the AVCodecID to @file{codec_id.h}? | ||
| When adding new codec IDs, also add an entry to the codec descriptor | ||
| list in @file{libavcodec/codec_desc.c}. | ||
|
|
||
| @item | ||
| If it has a FourCC, did you add it to @file{libavformat/riff.c}, | ||
| 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. | ||
|
|
||
| @item | ||
| Did you add an entry to the table of supported formats or codecs in | ||
| @file{doc/general_contents.texi}? | ||
|
|
||
| @item | ||
| Did you add an entry in the Changelog? | ||
|
|
||
| @item | ||
| If it depends on a parser or a library, did you add that dependency in | ||
| configure? | ||
|
|
||
| @item | ||
| Did you @code{git add} the appropriate files before committing? | ||
|
|
||
| @item | ||
| Did you make sure it compiles standalone, i.e. with | ||
| @code{configure --disable-everything --enable-decoder=foo} | ||
| (or @code{--enable-demuxer} or whatever your component is)? | ||
| @end enumerate | ||
|
|
||
|
|
||
| @chapter Patch submission checklist | ||
| @section Patch submission checklist | ||
|
|
||
| @enumerate | ||
| @item | ||
|
|
@@ -927,10 +877,61 @@ Test your code with valgrind and or Address Sanitizer to ensure it's free | |
| of leaks, out of array accesses, etc. | ||
| @end enumerate | ||
|
|
||
|
|
||
| @section Submission procedures | ||
|
|
||
| Patches should be posted to the | ||
| @uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} | ||
| mailing list. | ||
|
|
||
| There are several ways how this can be accomplished: | ||
|
|
||
| @subsection Using @code{git send-email}: Git sends e-mail directly via SMTP | ||
|
|
||
| @code{git send-email} will properly format and send | ||
| send patches without requiring extra care if the required setup is feasible. | ||
|
|
||
| Please see @url{https://git-send-email.io/}. | ||
|
|
||
| For gmail additionally see @url{https://shallowsky.com/blog/tech/email/gmail-app-passwds.html}. | ||
|
|
||
|
|
||
| @subsection With @code{git format-patch}: Sending patches from email clients | ||
|
|
||
| You can create patch files via @code{git format-patch} and send them as inline | ||
| text or as base64-encoded attachments, so your patch is not trashed during | ||
| transmission. Also ensure the correct mime type is used | ||
| (text/x-diff or text/x-patch or at least text/plain). | ||
|
|
||
| If you send several patches, send each patch as a separate mail, | ||
| do not attach several unrelated patches to the same mail. | ||
|
|
||
| You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show up, its mime type | ||
| likely was wrong. | ||
|
|
||
|
|
||
| The following trick allows to send patches via email clients in a safe | ||
| way. It has been tested with Outlook and Thunderbird (with X-Unsent | ||
| extension) and might work with other applications. | ||
|
|
||
| Create your patch like this: | ||
|
|
||
| @verbatim | ||
| git format-patch -s -o "outputfolder" --add-header "X-Unsent: 1" --suffix .eml --to [email protected] -1 1a2b3c4d | ||
| @end verbatim | ||
|
|
||
| Now you'll just need to open the eml file with the email application | ||
| and execute 'Send'. | ||
|
|
||
|
|
||
| @chapter Patch review process | ||
|
|
||
| All patches posted to ffmpeg-devel will be reviewed, unless they contain a | ||
| clear note that the patch is not for the git master branch. | ||
| Your patch will be reviewed on the mailing list. You will likely be asked | ||
| to make some changes and are expected to send in an improved version that | ||
| incorporates the requests from the review. This process may go through | ||
| several iterations. Once your patch is deemed good enough, some developer | ||
| will pick it up and commit it to the official FFmpeg tree. | ||
|
|
||
| Reviews and comments will be posted as replies to the patch on the | ||
| mailing list. The patch submitter then has to take care of every comment, | ||
| that can be by resubmitting a changed patch or by discussion. Resubmitted | ||
|
|
@@ -940,6 +941,9 @@ simple and small patches happen immediately while large patches will generally | |
| have to be changed and reviewed many times before they are approved. | ||
| After a patch is approved it will be committed to the repository. | ||
|
|
||
| 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. | ||
|
|
||
| We will review all submitted patches, but sometimes we are quite busy so | ||
| especially for large patches this can take several weeks. | ||
|
|
||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.