Re: [Rhythmbox-devel] New plugin: Pitch/Speed Shifting

[Sean McNamara <smcnam gmail com>]

Hi Jonathan,

On Sun, Nov 8, 2009 at 4:06 PM, Jonathan Matthew <jonathan d14n org> wrote:
> On Wed, Oct 28, 2009 at 12:42 AM, Sean McNamara <smcnam gmail com> wrote:
>
> [not ignoring specifics of this plugin this time]
>
>> Hello,
>>
>> I have started work on a new plugin for Rhythmbox. I have written it
>> in Vala, and developed it on / tested it against a Fedora 12 Beta
>> x86_64 system with Rhythmbox 0.12.5 and Vala 0.7.7.
>>
>> The current project status is that it works on my system, and the
>> primary features I wanted are implemented and functional. There is no
>> tarball release; instead, use git:
>> http://tiyukquellmalz.org/cgit/rbpitch <--- CGit web ui.
>> git clone git://tiyukquellmalz.org/rbpitch <--- git command to check
>> out the source.
>>
>> First, pictures are worth a thousand words. Here are some screenshots
>> -- I couldn't get window borders to show up, but each screenshot is of
>> a toplevel window.
>>
>> Addition to Rhythmbox Shell is visible in a red box (the box is not
>> rendered in the GUI): http://tiyukquellmalz.org/rbpitch-icon.png
>>
>> Clicking the icon in the rb shell pops up this window. First, with the
>> sliders in the default position:
>> http://tiyukquellmalz.org/rbpitch-ui-1.png
>> Second, with the sliders moved around a bit:
>> http://tiyukquellmalz.org/rbpitch-ui-2.png
>
> The problem here is that the useful range for the sliders is around
> 50% to 150%, but that's only 10% of the total range. I'd suggest
> making the sliders longer, and making them non-linear. The difference
> between 100% and 110% is far more important than the difference betwen
> 300% and 310%, but they're currently represented by the same amount of
> space on the slider.

These are some good suggestions. I also got the advice from Olli
Parviainen (author of PaceMaker) to express the Pitch scale in
semitones, and to center the tempo/speed scales on 0%. So 0% means the
sound passes through completely unmodified, 100% means it would be
shifted by a factor of 2, etc.

I took another look at PaceMaker in light of that email I got from
Olli. Here is how PaceMaker's UI works:

Speed and Tempo sliders: -50% to +100%, with 0% meaning unmodified sound
Pitch slider: -12.0 Semitones to +12.0 Semitones, with +0.0 St meaning
unmodified sound.

All three scales are linear. However, since the ranges are more
restricted than the current ones in rbpitch, it's actually quite
reasonable to find the values you want without typing in the
SpinButton.

Olli was kind enough to give me an algorithm for translating between
pitch % values and semitones. I think I'll keep the SpinButtons
automatically synchronized to the HScales using a shared Adjustment,
just like I'm doing now, but I'll modify the min/max ranges of the
Adjustments so that they match the values in PaceMaker. In the
adjustment value_changed signal handler, I make the translation
between the user-displayed value and the gstreamer value.

The only downside (if you can call it that) of this method is that you
can't shift the music into values that are in theory supported by the
backend. I can't really think of any use case (besides curiosity) that
justifies shifting below -50% or above 100%. On the other hand,
changing the pitch to semitones and reducing the range of the values
will make it easier for people with an actual use case (such as guitar
practice) to use this plugin effectively.

Even if this turns out being a bad idea, and even if you think we
should deviate from PaceMaker's design in the long run, I'm going to
drop a git commit today or tomorrow with the changes I just described.
I don't imagine it will be too difficult because I have the formula
for semitones from Olli. I will send another message to the list when
I've made the changes.

>
>> Now that I am happy with the functionality of the plugin, my new goal
>> is to help it reach the largest audience possible. I have chosen the
>> most favorable license possible by using the same one as the Rhythmbox
>> core, and I offer my help and perpetual maintainership of this plugin.
>> The RB maintainers can choose to help me reach the largest audience
>> possible, or not -- either way, I am still happy using the plugin
>> myself. I do believe it is of some general use, and there is precedent
>> for other media players including this functionality by default.
>
> Now that I've fixed the playback pipeline filter functionality so you
> can add and remove filters without changing playback state (mostly),

Excellent -- thank you! I just read your commit messages in git
master, and it looks like you expect it to be able to insert/remove an
element during playback, but there's still a bug removing while
paused. Hopefully you can work that out at some point before the next
release.

Also, would you recommend that I remove the code in my plugin that
stops the ShellPlayer's playback before inserting/removing the
element? This will introduce a bug in my plugin (hanging Rhythmbox if
you insert my plugin during playback) if anyone compiles it
out-of-tree for RB 0.12.5, but if you plan on including rbpitch in the
next release, it won't matter anyway. That would be the preferred
course for me. :)

> the main obstacle for this plugin is that the 'pitch' element doesn't
> appear to do timestamping, duration queries, or position queries
> properly, so the playback position slider advances at one second per
> second regardless of the actual rate. I haven't actually tried seeking
> but I'm guessing there are likely to be problems there too.

OK, I've triaged this, and it seems that your findings about the time
elapsed are correct. However, I also tried seeking. Seeking is indeed
messed up. If you have a song 1 minute long (without tempo shifting),
and you turn on shifting, then seeking to the position on the HScale
that represents 30 seconds elapsed without shifting will actually put
you at the position of 30 seconds elapsed _with_ shifting. So if I
shifted it up 50% faster, 30 seconds elapsed will have played through
45 seconds of the original song, or 75% completed. This is a bit
non-intuitive and is clearly a bug. I think this is actually more
problematic than the simple display bug in the case where you don't
seek.

Despite this fact, I am able to listen to the entire song from start
to finish while changing the tempo or speed. If I speed it up,
whenever the song ends, the filesrc sends an EOS and the next track
starts, but the progress bar for the time elapsed in the song is
indeed incorrect. If I slow it down, RB at least lets the entire
(slow) song finish, but the time elapsed progress bar will remain at
the end, and the time elapsed display will not count up, until it gets
EOS.

This is a UI oddity that anyone using the tempo or speed shifting
features of the plugin will have to live with for now. It seems to
have zero effect on the playback pipeline itself. Regardless, I will
try to work with the author of the pitch element to fix these bugs,
even though I have rather limited knowledge of gstreamer's internals
-- I am a gstreamer user, not a gstreamer hacker :) Since I can not
give an estimate of when this might be fixed upstream, I would prefer
that it not delay rbpitch unnecessarily, especially because this is
not a bug in rbpitch, but in gstreamer-plugins-bad.


One more thing: if you have decided to include rbpitch in the upstream
RB sources at some point, I will clone RB's source tree on my git
repository, commit my plugin and patches to configure.ac &
Makefile.am, and submit a pull request. This should make it easy for
you to grab exactly the diffs (vs. git master) to include rbpitch. I
know you are waiting on my changes to the sliders, so I'll get that
done first.

Thanks,

Sean

> _______________________________________________
> rhythmbox-devel mailing list
> rhythmbox-devel gnome org
> http://mail.gnome.org/mailman/listinfo/rhythmbox-devel
>