| Moving around plug-ins between source modules | |
| --------------------------------------------- | |
| Last updated: 2013-12-13 | |
| Contents: | |
| 1. How to get your plug-in out of -bad and into -good or -ugly (ie. policies) | |
| 2. How to move plugins between modules with git (ie. technical howto) | |
| ============================================================== | |
| 1. How to get your plug-in out of -bad and into -good or -ugly | |
| ============================================================== | |
| Since GStreamer 0.9.x, we have four plugin modules: -base, -good, -ugly, | |
| and -bad. Plug-ins are by default added to -bad. They can only move | |
| to -good or -ugly if a number of conditions are met: | |
| PEOPLE | |
| ------ | |
| - People involved: | |
| - There should be a person who is actively going to maintain this element; | |
| presumably this is the person writing the plug-in in the first place | |
| and opening the move request | |
| - There should be a GStreamer hacker who is willing to sponsor the element; | |
| this would be someone who is going to help out getting all the conditions | |
| met, act as a mentor if necessary,... | |
| - There should be a core developer who verifies that the checklist is | |
| met | |
| The three roles can be filled by two people, but not just one. | |
| In addition, an admin needs to perform the actual move, which involves | |
| CVS surgery. | |
| PROCESS | |
| ------- | |
| - bug in bugzilla gets filed by someone requesting a move from bad | |
| to good/ugly | |
| This is "requesting" the move. | |
| - a second person reviews the request and code, and verifies that the | |
| plugin meets the checklist items below, by commenting on the bug | |
| and giving a rundown of what still needs to be done | |
| This is "sponsoring" the move. | |
| - when the checklist is met, a third person can approve the move. | |
| This is "approving" the move. | |
| - an admin performs the move. | |
| This is "performing" the move. (Are you laughing yet ?) | |
| CHECKLIST | |
| --------- | |
| - The plug-in's code: | |
| - should descend from an applicable base class if possible | |
| - make use of G_DEFINE_TYPE macros | |
| - conform to the GStreamer coding style | |
| - use a custom debug category | |
| - use GST_(DEBUG/*)_OBJECT | |
| - use dashes in object property names to separate words | |
| - use correct value, name, nick for enums | |
| - use underscores in macros/function names/structs | |
| e.g.: GST_BASE_SINK, GstBaseSink, gst_base_sink_ | |
| - use g_assert(), g_return_if_fail(), g_return_val_if_fail() for pre/post | |
| condition checks | |
| - must not have any functional code within g_assert(), g_return_if_fail() or | |
| g_return_val_if_fail() statements, since those would be turned into NO-OPS | |
| if the code is compiled with -DG_DISABLE_CHECKS (as is often done on | |
| embedded systems). | |
| - The plug-in's build: | |
| - should be correctly integrated with configure.ac | |
| - files implementing elements should be named according to their class name, | |
| e.g GstBaseSink -> gstbasesink.c | |
| - should list libs and cflags in stack order, with lowest in the stack first | |
| (so one can link against highest in the stack somewhere else without | |
| picking up everything from the somewhere else) | |
| e.g. $(GST_PLUGINS_BASE_CFLAGS) \ | |
| $(GST_BASE_CFLAGS) \ | |
| $(GST_CFLAGS) $(CAIRO_CFLAGS) | |
| - The compiled plug-in: | |
| - should show up correct in gst-inspect output; no warnings, no unknown | |
| types, ... | |
| - The plug-in should be put in the correct location inside the module: | |
| sys/: plug-ins that include system headers/link to system libraries; | |
| usually platform-dependent as well | |
| name after whatever system "thing" they use (oss, v4l, ...) | |
| gst/: plug-ins with no external dependencies, only GLib/GStreamer/liborc | |
| ext/: plug-ins with external dependencies | |
| - The plug-in is documented: | |
| - the compiled-in descriptions (element details) should be correct | |
| - every element in the plug-in should have gtk-doc documentation: | |
| - longer description of element | |
| - why you would use this element | |
| - example launch line OR example source code | |
| (for example, see | |
| http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-plugins/html/gst-plugins-base-plugins-audiotestsrc.html | |
| for the first and | |
| http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-good-plugins/html/gst-plugins-good-plugins-level.html | |
| for the second) | |
| - if the element has custom messages, they should be documented | |
| - signals and properties should be documented | |
| - The plug-in should come with tests: | |
| - preferably, a unit test should be written, testing things like: | |
| - setup and teardown | |
| - push in buffers in all supported formats and verify they are handled | |
| properly | |
| - push in buffers that trigger error cases, and verify errors are | |
| correctly thrown | |
| for example, see gst-plugins-base/check/elements/audioconvert | |
| The unit test should be put in check/elements/(nameofelement) | |
| and be added to check_PROGRAMS and Makefile.am | |
| - if a unit test is not appropriate (for example, device elements), | |
| a test application should be written that can be run manually | |
| - The tests should be leak-free, tested with valgrind | |
| - the unit tests in check/ dirs are valgrinded by default | |
| - the manual tests should have a valgrind target | |
| - leaks in the supporting library (and verified to be in the supporting | |
| library !) can be added to suppression files | |
| - The elements should not segfault under any circumstance. This includes: | |
| - wrong pipelines | |
| - bad data | |
| - The element must not rely on a running GLib main loop, meaning it can't | |
| use g_idle_add(), g_timeout_add(), g_io_add_watch(), etc. | |
| - The plugins need to be marked correctly for translations. | |
| - All error conditions should be correctly handled using GST_ELEMENT_ERROR | |
| and following practice outlined in | |
| http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-GstGError.html | |
| - this includes: | |
| - message strings need to be marked for translation | |
| - should be short, well-written, clear | |
| - in particular, should *not* contain debug info, strerror, errno, ... | |
| No, really ! NO STRERROR, NO ERRNO. If you are too lazy to provide | |
| the user of your library with a nice experience, put your crap in | |
| the debug string | |
| - Decision should be made if it should go into good (LGPL license, | |
| LGPL dependencies, no patent issues) or ugly | |
| - plugin documentation needs to be added: | |
| - see gstreamer/docs/README; section on adding plugins and elements | |
| - "make update" in docs/plugins and commit the new file | |
| - edit -docs.sgml and add an include for the file | |
| ============================================ | |
| 2. Moving plugins with GIT (technical howto) | |
| ============================================ | |
| Let's say we are moving the 'weneedthis' plugins from -bad/gst/weneedthis/ to | |
| -good. | |
| We want to end up with the following situation after the plugin move: | |
| * weneedthis is no longer usable/visible in HEAD of -bad | |
| * weneedthis is present and usable in HEAD of -good | |
| * The whole history of commits concerning weneedthis are visible in -good | |
| * The whole history of commits concerning weneedthis are visible in -bad | |
| * If we go back to the previous release commit for -good, the 'weneedthis' | |
| plugin code and commit history are not visible. | |
| * If we go back to the previous release commit for -bad, the 'weneedthis' | |
| plugin code and commits are visible. | |
| # Four steps for moving plugins | |
| ---------- | |
| For this, we use git-format-patch which will extract the various commits | |
| involving the plugin we want to move into individual numbered patches. | |
| > cd gst-plugins-bad.git/ | |
| > git format-patch -o ../patches/ --subject-prefix="MOVED FROM BAD" start..HEAD -- gst/weneedthis/ tests/check/elements/weneedthis.c | |
| or (without the prefix) | |
| > git format-patch -o ../patches/ --no-numbered --subject-prefix='' start..HEAD -- gst/weneedthis/ tests/check/elements/weneedthis.c | |
| We can then take those patches and commit them into -good. | |
| For safety, it is recommended to do all this work in a temporary branch | |
| > cd ../gst-plugins-good.git/ | |
| > git checkout -b moving-weneedthis | |
| > git am -k ../patches/*.patch | |
| At this point, we have all the commits related to gst/weneedthis/ in -bad. | |
| We also need to move the other related parts in: | |
| * configure.ac | |
| * Makefile.am from intermediary directories (if needed) | |
| * i18n | |
| * documentation | |
| All those modifications should be committed as one commit with an obvious | |
| summary: | |
| "Moved 'weneedthis' from -bad to -good" | |
| > git commit -v -m "Moved 'weneedthis' from -bad to -good" <modified files> | |
| We can now merge the branch into master and push it out for everybody. | |
| > git checkout master | |
| > git merge moving-weneedthis | |
| The last step is to remove the plugin from the original module: | |
| > git rm gst/weneedthis/ | |
| Remove all the code that was moved from configure, makefiles, documentations, | |
| and commit that with the *SAME* commit message as the last commit we did in | |
| -good: | |
| > git commit -v -m "Moved 'weneedthis' from -bad to -good" <modified files> | |