No pitch compensation when playing audio with playbackRate != 1

Bug Reports
#1

REPRODUCIBILITY (% or how often): 100%
BUILD ID: Sailfish OS 3.3.0.16 (Rokua)
HARDWARE (Jolla1, Tablet, XA2,…): Sony Xperia X
UI LANGUAGE: Ger
REGRESSION: (compared to previous public release: Yes, No, ?): maybe

DESCRIPTION:

When playing audio aat higher playbackRates than 1 with the QMediaPlayer the pitch goes up, below 1 it goes down. The pitch should be compensated.

PRECONDITIONS:

have a qmediaplayer

STEPS TO REPRODUCE:

  1. play audio
  2. change playback rate

EXPECTED RESULT:

pitch stays the same and audio is faster/slower

ACTUAL RESULT:

pitch goes up/down

ADDITIONAL INFORMATION:

This issue might have been introduced with SFOS 3.3
It looks like a gstreamer plugin is missing which should do the pitchcompensation in the gstreamer pipeline.

Affected Apps: Talefish, Gpodder (with playbackRate patch)

6 Likes
[Release notes] Verla 4.2.0
#2

This is the first time I read about QMediaPlayer from Qt 5.6 having pitch compensation. Do you have any reference for that?

3 Likes
#3

It’s only in Qt releases newer then what Jolla ships atm.
One of the features I’d love to see and use in future releases.

1 Like
#4

I opened a topic about this for the next meeting. I named you, @Keeper-of-the-Keys , as a substitute, hope thats okay.

#5

I am not sure I can make it but will try

#6

Reposted for next meeting: Community meeting on IRC 11th Mar 2021

#7

In the latest IRC meeting topic, you suggested that it should be done by adding a pitch-adjustment element to the pipeline, but our version of QtMultimedia doesn’t allow that. Is that something that could always be present in the pipeline, but basically just passthrough if it’s not set to change the pitch? I’m wondering if that could be added to QtMultimedia, so you wouldn’t need to modify the pipeline,

#8

Hi, sorry for not attending to the meeting and thanks for being open to discuss this here.
scaletempo could be added to the pipeline and automatically pitch all playbacks.
I would even say that this would be a good default, as for a mediaplayer I’d expect, that if i change the speed, the audio is pitched correctly.
However, this is a breaking change. I don’t know if the speed property is used in some places to pitch the sound on purpose, if yes, this would introduce a regression.

If you think that is not the case, I think it would be the simplest solution to add scaletempo to the default pipeline.

1 Like
#9

@abranson any updates on this?

#10

Sounds like it might work, and I doubt anything relies on the pitch rising or falling. Did you try anything out yet?

#11

@abranson I’d love to. But I guess I need to modify qt and build it myself. Can you point me to documentation on how to do that?

I’d love to see this feature as soon as possible released and am willing to help. So let me know if I can do anything.

#12

According to this it seems like after 1.4.2 you can use a flag on playbin, before you need to use scaletempo. (We’re on 1.4.1)

And it looks like scaletempo could be added here

I’ll do further investigation soon

#13

I tried to do the changes and compiled it with the platform sdk and the mer-5.6 branch, but had no success yet.
The changes should be simple, but I’m not sure I did the right things to insert scaletempo as a new element into the pipeline…
However, my compilation also is different from what i have on my phone, at least the sizes are vastly different, so I am not sure if i added some linking problems with that now…

Is there a chance, that someone with experience with this sort of stuff could take a look, @abranson ?

#14

Did you push your changes somewhere so I can give them a spin?

#15

My naive try fits here :wink:

I hope it doesn’t show too clearly, that I had no idea what I’m doing :smiley:

I tried to imitate what i found at openjfx

$  git diff
diff --git a/src/plugins/gstreamer/audiodecoder/qgstreameraudiodecodersession.cpp b/src/plugins/gstreamer/audiodecoder/qgstreameraudiodecodersession.cpp
index 3360583c..89e6c1da 100644
--- a/src/plugins/gstreamer/audiodecoder/qgstreameraudiodecodersession.cpp
+++ b/src/plugins/gstreamer/audiodecoder/qgstreameraudiodecodersession.cpp
@@ -96,10 +96,16 @@ QGstreamerAudioDecoderSession::QGstreamerAudioDecoderSession(QObject *parent)
         // Set the rest of the pipeline up
         setAudioFlags(true);
 
+        GstElement* scale = gst_element_factory_make("scaletempo", nullptr);
+        GRefPtr<GstPad> pad2 = adoptGRef(gst_element_get_static_pad(scale, "sink"));
+
         m_audioConvert = gst_element_factory_make("audioconvert", NULL);
 
         m_outputBin = gst_bin_new("audio-output-bin");
-        gst_bin_add(GST_BIN(m_outputBin), m_audioConvert);
+        gst_bin_add_many(GST_BIN(m_outputBin), m_audioConvert,scale);
+
+        gst_element_add_pad(m_outputBin, gst_ghost_pad_new("sink", pad2.get()));
+         gst_object_unref(GST_OBJECT(pad2));
 
         // add ghostpad
         GstPad *pad = gst_element_get_static_pad(m_audioConvert, "sink");
1 Like
#16

Does this work?

https://build.sailfishos.org/package/show/home:abranson:sailfishapps/qtmultimedia

3 Likes
#17

yes! that one did the trick, couldnt install the rpm but copying the file worked.

This is just such a great relief :slight_smile: Thanks a lot. For anyone interested, this is the diff. Glorios 6 lines of code. I’m hyped :smiley:

2 Likes
#18

Great! I’ll PR that and see if any other sailors can see potential problems. Might be upstreamable even? I doubt that pitch adjustment through playback speed is useful for anyone. There are better ways to adjust the pitch, and I think scaletempo gives the expected behaviour with seek.

3 Likes
#19

It looks like it didnt make it into this release? Hope its going into the next.

#20

Yeah, all this was too late to get in before branching of 4.1.0, and wasn’t really critical enough to get cherry-picked into it. It’s definitely in the next release.

3 Likes