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>