Re: Patch cplay to use mplayer with speed-control
Hi Daniel,
> Hi everyone,
>
> I tried patching cplay to use mplayer(>=1.0pre7) which can also do
> speed-control on audio-files. My patch creates and uses a fifo to control
> mplayer and adds some keybindings to cplay (decr: [, incr: ], reset: =).
>
> I tried to write the patch as clean and stable as possible and provide my first
> trial on my webpage. I would be glad to receive some reviews and comments how to
> improve my code quality and some ideas what to do with my patch:
> - Should I mail it to the author of cplay?
> - Should I mail it to the maintainer of the debian package?
> - Should I fill a bugreport "feature-request" providing my patch?
Mplayer still isn't in the Debian archive. So introducing a dependency
upon it would require cplay to be moved from "main" into "contrib",
unless the dependency is optional. If it is optional (that is, if you
improve your patch to test for an mplayer binary at runtime), you could
certainly consider submitting it to the Debian maintainer or to
upstream. Probably upstream would be better, assuming the package in
Debian isn't very much changed from upstream's version.
Regarding your patch at http://www.argafal.de/public/patchcplay :
- It would be more readable if you use unified diff format ("diff -u")
to show some lines of context.
- The code where you create the fifo makes it with a predictable
filename; also, it assumes that if it (or another file of the same name)
already exists, it may be deleted by the current cplay user. Hence it
is susceptible to a very trivial DOS attack. You ought to fix this
before submitting the patch.
I don't know Python well so I imagine others may also have comments.
best regards,
--
Kevin B. McCarty <kmccarty@princeton.edu> Physics Department
WWW: http://www.princeton.edu/~kmccarty/ Princeton University
GPG: public key ID 4F83C751 Princeton, NJ 08544
Reply to: