Development: Hints for Refactoring Existing Drivers

From LinuxTVWiki
Jump to navigation Jump to search

Free software often grows over time -- both in terms of the amount of code and as well as the number of individuals contributing to the code base. In fact, with free and open source software, over the course of the project's development life, it is quite common for other people to become involved simply because the original author(s) may not always be available or even willing to maintain the code forever. This holds true for V4L-DVB drivers and associative utilities. Yet, in our case, we'd like to stress that you don't need to be a core developer in order to become involved in our software's continued progression, as everybody is invited to review the code and contribute. All in all, contributions can be done on several different levels that vary in complexity; for example:

  • patches to the code for the purposes of providing
    • simple clean-ups (such as remedying spelling/typo/grammar/punctuation errors or coding style violations)
    • more detailed bug fixes and/or logical corrections
  • more involved changes that introduce driver performance optimisations
  • introduction of new feature sets to the guiding Linux Media Infrastructure API
  • a complete refactoring of portions of the V4L-DVB code base, typically done in light of simplifying the code, and perhaps making it more future-proof

What is Refactoring?

Wikipedia provides a brief definition and discussion of the refactoring process, in which you will find mentioned that the basic principles espoused are:

  • Simplify your Code.
  • Don't introduce new Features in the Refactoring Process.
  • Break down API Barriers where they don't make Sense
  • Introduce new Interfaces where they stabilize your Code.
  • Try to keep external Interfaces as stable as possible.
  • Split complex Modules into smaller, easily testable ones with well-defined Tasks.
  • Avoid Confusion.
  • Simplify your Code.

Refactoring is a natural process in respect to projects that usually grow over time. To use an analogy: every now and then, what has grown through natural means into a jungle will need to be cut back in order to make it a nice place to live again. Unfortunately, often due to clashes of vanities and feelings of possessiveness, the steps towards a refactoring can sometimes be accompanied by loud discussions that equate to little more than unruly philosophical debates and holy wars. But don't worry: dogs that make noise usually don't bite. Revolutionary ideas sometimes need some fighting to get through.

Since developers may become, with time, a little myopic in respect towards their own code, it makes sense that the reviewer/refactorer and programmer are often not the same person. No part of the Linux kernel is exclusively owned by a maintainer. If a new solution makes sense, sooner or later it will make it into the public source tree.

Refactoring does not mean new features. Refactoring means that you make the code maintainable.

Keep it Simple

The simpler the resulting code is, the easier it will be to maintain. Even from the performance point of view, simple code is almost always preferable.

If you want to see some impressive code following this approach, you may want to check out one of Fabrice Bellards Projects, e.g. TinyGL or the ID software GPL games Doom, Quake & Co -- both of these are great examples of highly efficient coding (in terms of both compactness and performance).

So please: Whatever you do, please keep it Simple.

Please Never Ever Try to Apply Big Changes All in One Shot

If you plan to rewrite bigger portions of the code please don't create a huge patch or thousands of patches. Please avoid postings like "[PATCH] I ported all drivers to this or that kernel API because this is the way the kernel maintainers like it to see!". Instead, fork the relevant code modules into a separate development repo so that people can easily test them concurrently with the old code and are free to continue to use the old, usually well-tested, code until your new version has matured.

That practice wasn't always adhered to during the history of V4L-DVB code development and, unfortunately, there were several case or situations during which some elements of the code base were barely usable for weeks or even months because of neglecting the principle laid out above.

The bottom line is that you should respect that other people are using V4L-DVB software in productive environments and may rely upon a working code base. In addition, one may never be able to foresee the effect of major changes on all the exotic platforms the V4L-DVB stack is used upon. If you really think using new APIs is the way to go then please let's fork the source tree and move over to the new tree as soon it is well-tested and verified upon all architectures.

Addendum: The Psychological Prerequisites for Refactoring

Refactoring is a complex process and does not simply mean rewriting the existing code. Rather, it also involves trying to convince other developers to accept your changes, and, it is this later point that is perhaps the much more difficult part of the process. We encourage you to rise up to that challenge and present two such approaches you might take in that regard:

Approach #1: The Democratic Trial -- let's discuss!

  • Somebody starts the discussion and makes his or her prosaic proposal.
  • nothing happens for a while.
  • Guru #1 says: "mmhh... interesting!"
  • Guru #2 says: "No go!!! we need to do it like I already started years ago!!"
  • Guru #3 says: "yes, Guru #2 is right, we have to follow the mainstream API development!"
  • Guru #1 annotates that the new approach has some appealing properties and simplifies many things a lot.
  • several less involved developers join discussion and take sides with Guru #1, #2 or Guru #3.
  • The Proposer refines his approach and maybe even provides some sample code.
  • Guru #1 says that he likes it.
  • Guru #2 or Guru #3 make their own proposal and post some sample code explaining the plan they are following since years.
  • Guru #1 calls it bloat and emphasizes how simple the new approach may be.
  • Guru #3 says that we would have to rewrite a lot of code.
  • (mails get harsh in sound, sometimes even quite personal. Arguments count less and less.)
  • The Proposer, Guru #2 and Guru #3 post several new proposals converging closer to each other. The sample code gets more and more complicated due to all the special cases that come to discussion.
  • For a while it becomes quiet.
  • Guru #2 or Guru #3 posts a new approach that funnily looks quite similar in concept to the original code of the proposer but is a little less clean due to the exhausting fights.
  • Proposer says: "hey, that looks familiar!" -- Guru: "may be."
  • Sometimes a few mails refining the final details close the discussion.

Approach #2: Just do it

The process outlined above may sound very democratic and entertaining, but it eats up a lot of time and can seriously hamper good spirits/mood, and, thus, can be kind of counterproductive. If you don't want to spend your energy in those regards, or risk being sucked into flame wars, you may want to try this:

  • Implement your approach for 2 or 3 sample drivers -- perhaps showcasing your changes with a complex driver and as well with two more simpler cases
  • Then post your code changes on the Linux-Media Mailing List and propose to start a new development branch where this work can be further continued across the entire V4L-DVB code base. If you don't already have a development repo with the V4L-DVB project from which you can work, one can easily be provided if you request (though this is certainly not a requirement).

If your code revisions have genuine, and easily provable, advantages over the older code, it is highly likely that your implementation will be approved of and, ultimately, incorporated within the V4L-DVB source.

While this approach runs somewhat in opposition to the "Democratic Discussion Process", like that outlined in the prior section, it will, however, in most cases, take a whole lot less time to just write the new code and present it, as described above, then having to await the end of any lengthy discussions.

In addition, this latter approach may be able to steer you, and the community at large, clear away from any sort of nefarious activities that may prevent the emergence of homogeneous and easy-to-maintain code. You are dealing with fellow human beings and, unfortunately, you can not account for all of their behaviour to be gracious or dignified. The democratic approach introduces the risk that discussions will devolve into harsh words being used as egos may be hurt due to one code base being rejected or replaced by another. Such occurrences can then spawn machinations within some individuals -- they may later try to "personalize" the code base just because they can't resist putting in some pieces of their old theory just to keep it alive, and nobody within the community asks why they're doing it simply because everybody fears or is un-interested in becoming involved in yet another discussion with that person which is bound to degenerate into a new flame war.

On a much more positive note, there may be a beneficial side effect for the development community at large from your having adopted the approach outlined within this section. Specifically, any rational developer will be highly motivated to make their own code as elegant as your own reference code.

Lastly, bear in mind that the V4L-DVB project is contributed to by anyone who wants to join development, and that no one person or guru, so to speak, has absolute power over the code. So take heart in the fact that everything that makes sense will sooner or later make it into the mainstream V4L-DVB source tree.