Cutify #28

mardy · 2021-05-07T22:50:27Z

This is now ready for review. I've tested a few use cases, and everything seems to work fine. Playing remote media also works.

I've removed the unit tests, which were not testing much anyway, and replaced them with functional tests where the whole media-hub-server component is run in a standalone D-Bus session where all dependent services (like powerd, the power indicator, telepathy) are mocked.

Fixes: #29
Fixes: #24

Signal handling is not yet done.

This gets rid of all threading (except the one in gstreamer itself, which is not exposed to the consuming code).

https://bugreports.qt.io/browse/QTBUG-93405 is making everything more complicated, but let's optimize signal emission later.

This greatly reduces the number of signals emitted on D-Bus.

Now we always know that we have a player bound to it.

Remove a couple of entries which change dynamically and cause frequent emissions of the PropertiesChanged signal: https://bugs.launchpad.net/ubuntu/+source/media-hub/+bug/1598799 These properties are not part of the spec and are not read by other processes anyway.

This will allow upstart (or systemd) to restart our service.

This was unused even in the original version.

This is only testing the gstreamer engine: we'll test it in the functional tests.

These files were all unused. Let's start from scratch.

They'll not be used by the client.

This is hardly relevant here, and removing it can make the tests run a bit faster.

Fixes: ubports#24

This can confuse the client.

Clients wishing to play alert sounds (such as unity8 when taking a screenshot) need to set this in order not to stop the media player.

If the server is not closed, tests fail in CI.

Set the context object so that the signal will be automatically disconnected when the object dies.

For some reason sometimes using localhost fails in my local machine, whereas the numeric address always works.

Let's still keep the URL-based detection for a while, until we fix the client library as well.

lduboeuf · 2021-05-28T21:17:20Z

awesome work. how to test for remote media ?

lduboeuf · 2021-05-29T08:16:29Z

Confirmed it fix #24 :)

mardy · 2021-05-29T15:50:50Z

awesome work. how to test for remote media ?

You could try MiTubo; here's a very old click, let me know if you need arm64. I'll upload it to the store after the new media-hub gets released into some OTA.
Or, if you'd like to see the latest version, you can also build it yourself with clickable from here.

lduboeuf · 2021-05-31T08:19:02Z

@mardy , humm , had no luck to build mitubo project with clickable version 6.24.1

WARNING: Error setting up Qt for '/usr/bin/qmake': Cannot extract the mkspecs directory.
ERROR: /home/lduboeuf/dev/perso/ubuntutouch/mitubo/mitubo.qbs:12:5 Error while handling product 'MiTubo':
/usr/share/qbs/imports/qbs/base/QtApplication.qbs:32:5 Dependency 'Qt.core' not found for product 'MiTubo'.
/usr/share/qbs/imports/qbs/base/QtGuiApplication.qbs:32:5 Dependency 'Qt.gui' not found for product 'MiTubo'.
/home/lduboeuf/dev/perso/ubuntutouch/mitubo/mitubo.qbs:69:9 Dependency 'Qt.core' not found for product 'MiTubo'.
/home/lduboeuf/dev/perso/ubuntutouch/mitubo/mitubo.qbs:70:9 Dependency 'Qt.quick' not found for product 'MiTubo'.
/home/lduboeuf/dev/perso/ubuntutouch/mitubo/mitubo.qbs:71:9 Dependency 'Qt.quickcontrols2' not found for product 'MiTubo'.
/home/lduboeuf/dev/perso/ubuntutouch/mitubo/mitubo.qbs:72:9 Dependency 'Qt.svg' not found for product 'MiTubo'.

mardy · 2021-05-31T10:35:17Z

@mardy , humm , had no luck to build mitubo project with clickable version 6.24.1
[...]

I'll investigate why this happens. Meanwhile, please use v6.23.3, which works.

lduboeuf · 2021-05-31T12:34:02Z

@mardy well, same with 6.23.3, also the link provided for the click does not work here (http://mardy.it/archivos/tmp/it.mardy.mitubo_0.1-0_armhf.click)

lduboeuf · 2021-05-31T22:06:54Z

don't know if related but audio recording and playback doesn't work on messaging app:

Recording:

** (messaging-app:11017): CRITICAL **: gst_audio_format_from_string: assertion 'format != NULL' failed
ALSA lib conf.c:3814:(snd_config_update_r) cannot access file /usr/share/alsa/alsa.conf
[PERFORMANCE]: Last frame took 37 ms to render.

....

setMedia() media:  QUrl("file:///home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audioElXUfy.ogg")
virtual QMediaPlayer::MediaStatus AalMediaPlayerControl::mediaStatus() const 

Setting media to:  QUrl("file:///home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audioElXUfy.ogg")
Failed to set media  QUrl("file:///home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audioElXUfy.ogg") :  org.freedesktop.DBus.Error.UnknownObject: No such object path '/core/ubuntu/media/Service/sessions/0'

PlayBack:

void AalMediaPlayerService::play()
Failed to check if source is video:  org.freedesktop.DBus.Error.UnknownObject: No such object path '/core/ubuntu/media/Service/sessions/0'
Failed to get current playback position:  org.freedesktop.DBus.Error.UnknownObject: No such object path '/core/ubuntu/media/Service/sessions/0'

mardy · 2021-06-02T05:01:44Z

don't know if related but audio recording and playback doesn't work on messaging app:
[...]

In both cases, it looks like media-hub is not running (it might have crashed, maybe?). Can you please reproduce it while collecting the bustle logs (busctl --user monitor core.ubuntu.media.Service)?

lduboeuf · 2021-06-02T06:56:12Z

@mardy here is some logs:

Recording audio:

‣ Type=signal  Endian=l  Flags=1  Version=1  Priority=0 Cookie=2
  Sender=org.freedesktop.DBus  Destination=:1.178  Path=/org/freedesktop/DBus  Interface=org.freedesktop.DBus  Member=NameAcquired
  MESSAGE "s" {
          STRING ":1.178";
  };

‣ Type=method_call  Endian=l  Flags=0  Version=1  Priority=0 Cookie=24
  Sender=:1.175  Destination=core.ubuntu.media.Service  Path=/core/ubuntu/media/Service/sessions/0  Interface=org.mpris.MediaPlayer2.Player  Member=Stop
  UniqueName=:1.175
  MESSAGE "" {
  };

‣ Type=error  Endian=l  Flags=1  Version=1  Priority=0 Cookie=51  ReplyCookie=24
  Sender=:1.176  Destination=:1.175
  ErrorName=org.freedesktop.DBus.Error.UnknownObject  ErrorMessage="No such object path '/core/ubuntu/media/Service/sessions/0'"
  UniqueName=:1.176
  MESSAGE "s" {
          STRING "No such object path '/core/ubuntu/media/Service/sessions/0'";
  };

‣ Type=method_call  Endian=l  Flags=0  Version=1  Priority=0 Cookie=25
  Sender=:1.175  Destination=core.ubuntu.media.Service  Path=/core/ubuntu/media/Service/sessions/0  Interface=org.mpris.MediaPlayer2.Player  Member=OpenUri
  UniqueName=:1.175
  MESSAGE "s" {
          STRING "file:///home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audiopFCsjq.ogg";
  };

‣ Type=error  Endian=l  Flags=1  Version=1  Priority=0 Cookie=52  ReplyCookie=25
  Sender=:1.176  Destination=:1.175
  ErrorName=org.freedesktop.DBus.Error.UnknownObject  ErrorMessage="No such object path '/core/ubuntu/media/Service/sessions/0'"
  UniqueName=:1.176
  MESSAGE "s" {
          STRING "No such object path '/core/ubuntu/media/Service/sessions/0'";
  };

PlayBack audio:

Type=method_call  Endian=l  Flags=0  Version=1  Priority=0 Cookie=151
  Sender=:1.175  Destination=core.ubuntu.media.Service  Path=/core/ubuntu/media/Service/sessions/0  Interface=org.mpris.MediaPlayer2.Player  Member=OpenUri
  UniqueName=:1.175
  MESSAGE "s" {
          STRING "file:///home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audiokhsKOl.ogg";
  };

‣ Type=error  Endian=l  Flags=1  Version=1  Priority=0 Cookie=178  ReplyCookie=151
  Sender=:1.176  Destination=:1.175
  ErrorName=org.freedesktop.DBus.Error.UnknownObject  ErrorMessage="No such object path '/core/ubuntu/media/Service/sessions/0'"
  UniqueName=:1.176
  MESSAGE "s" {
          STRING "No such object path '/core/ubuntu/media/Service/sessions/0'";
  };

‣ Type=method_call  Endian=l  Flags=0  Version=1  Priority=0 Cookie=152
  Sender=:1.175  Destination=core.ubuntu.media.Service  Path=/core/ubuntu/media/Service/sessions/0  Interface=org.freedesktop.DBus.Properties  Member=Get
  UniqueName=:1.175
  MESSAGE "ss" {
          STRING "org.mpris.MediaPlayer2.Player";
          STRING "IsAudioSource";
  };

‣ Type=error  Endian=l  Flags=1  Version=1  Priority=0 Cookie=179  ReplyCookie=152
  Sender=:1.176  Destination=:1.175
  ErrorName=org.freedesktop.DBus.Error.UnknownObject  ErrorMessage="No such object path '/core/ubuntu/media/Service/sessions/0'"
  UniqueName=:1.176
  MESSAGE "s" {
          STRING "No such object path '/core/ubuntu/media/Service/sessions/0'";
  };

‣ Type=method_call  Endian=l  Flags=0  Version=1  Priority=0 Cookie=153
  Sender=:1.175  Destination=core.ubuntu.media.Service  Path=/core/ubuntu/media/Service/sessions/0  Interface=org.freedesktop.DBus.Properties  Member=Get
  UniqueName=:1.175
  MESSAGE "ss" {
          STRING "org.mpris.MediaPlayer2.Player";
          STRING "Position";
  };

mardy · 2021-06-02T19:15:31Z

Thanks @lduboeuf , it's great that you found this issue before we merged this!

So, there are actually two issues

media-hub could not parse the apparmor label messaging-app (enforce) and crashed. I have a fix for this and I'll commit it soon.
but even once the above is fixed, the recorded media won't be played back, before for some reason the file is detected by gstreamer as a video file:

    80.741 - unknown:0 - parsed_uri.path: /home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audioauqrdE.ogg
    80.741 - unknown:0 - 
    80.742 - unknown:0 - Updating MPRIS TrackList properties:
    80.742 - unknown:0 - 	Tracks: 0
    80.743 - unknown:0 - 	has_previous: 0
    80.743 - unknown:0 - 	has_next: 0
    80.752 - unknown:0 - Found content type: video/3gpp
    80.752 - unknown:0 - Found video content

Inspecting the file from the terminal gives a similar result:

phablet@ubuntu-phablet:~$ file /home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audioauqrdE.ogg
/home/phablet/.cache/com.ubuntu.messaging-app/MessagingApp/audioauqrdE.ogg: ISO Media, MPEG v4 system, 3GPP

So I think I'll just hack around this and consider the mime type video/3gpp as audio.

This fixes a crash when receiving a label like "messaging-app (enforce)".

These files are detected as having a video/3gpp MIME type. Since we know that they are audio files, let's make a little hack.

mardy · 2021-06-02T19:40:33Z

So, there are actually two issues
[...]

Both should be fixed now, please try the latest media-hub: https://ci.ubports.com/blue/organizations/jenkins/ubports%2Fmedia-hub/detail/PR-28/30/artifacts/

lduboeuf · 2021-06-02T20:10:11Z

@mardy confirm it works with messaging app 👌

UniversalSuperBox

I've run the Canonical test plans and run many apps which test the sound system. I think we're ready to go.

mardy added 2 commits April 30, 2021 13:10

Replace process-cpp with QCoreApplication

4982e49

Signal handling is not yet done.

ServiceSkeleton: remove unused private method

e943656

mardy force-pushed the cutify branch 6 times, most recently from 99870f0 to 20414da Compare May 10, 2021 18:40

dobey marked this pull request as draft May 10, 2021 20:11

mardy force-pushed the cutify branch from 20414da to c05f90c Compare May 10, 2021 21:17

mardy added 16 commits May 13, 2021 19:10

Rewrite service using Qt

daf6e2f

This gets rid of all threading (except the one in gstreamer itself, which is not exposed to the consuming code).

service: add DBusPropertyNotifier class

3828b47

service: move player registration to skeleton

df4fa1f

service: use DBusPropertyNotifier in Player adapter

782947d

service: register types used in the D-Bus interfaces

b21b7e8

service: rework registration of D-Bus objects

9443863

https://bugreports.qt.io/browse/QTBUG-93405 is making everything more complicated, but let's optimize signal emission later.

service: create an object to manage the MPRIS interfaces

8a41507

This greatly reduces the number of signals emitted on D-Bus.

service: simplify PlayerSkeleton

93570aa

Now we always know that we have a player bound to it.

service: terminate on disconnection from D-Bus

4ad47d1

This will allow upstart (or systemd) to restart our service.

service: remove unused type CoverArtResolver

26ba943

This was unused even in the original version.

tests: remove unit tests subdirectory

5cdc0c3

This is only testing the gstreamer engine: we'll test it in the functional tests.

tests: remove unused tests

4ebcd9a

These files were all unused. Let's start from scratch.

tests: move data files into service subdirectory

3ba035c

They'll not be used by the client.

service: do not crash if OpenUri is called with no params

914bbb5

service: simplify boolean logic in Player adaptor

df2332b

mardy force-pushed the cutify branch from 0b1db1f to 1ec637c Compare May 13, 2021 16:10

mardy mentioned this pull request May 13, 2021

Support playing remote media #22

Closed

mardy marked this pull request as ready for review May 13, 2021 16:15

mardy mentioned this pull request May 13, 2021

Basic HTTP authorization header support #19

Closed

mardy added 9 commits May 27, 2021 19:10

tests: do not use dbus-monitor ficture on http tests

d8d3e84

This is hardly relevant here, and removing it can make the tests run a bit faster.

service: allow the same client to play concurrent sounds

78bce8c

Fixes: ubports#24

service: do not emit Paused state when setting up the playbin

c8be24d

This can confuse the client.

service: workaround client library bug

bde55a2

service: allow the AudioStreamRole property to be written to

c90652a

Clients wishing to play alert sounds (such as unity8 when taking a screenshot) need to set this in order not to stop the media player.

tests: properly shutdown HTTP server

285c14d

If the server is not closed, tests fail in CI.

service: disconnect PlayerImplementation handlers

255c883

Set the context object so that the signal will be automatically disconnected when the object dies.

service: print error message from gstreamer

9663476

tests: use 127.0.0.1 instead of localhost

783ae81

For some reason sometimes using localhost fails in my local machine, whereas the numeric address always works.

mardy force-pushed the cutify branch from 2e2fe9d to ff60359 Compare May 27, 2021 16:10

service: get the number of audio/video streams from the playbin

45a49dd

Let's still keep the URL-based detection for a while, until we fix the client library as well.

mardy force-pushed the cutify branch from ff60359 to 45a49dd Compare May 27, 2021 17:50

mardy added 2 commits June 2, 2021 22:39

service: use aa_splitcon() to parse AppArmor profiles

6a28cfe

This fixes a crash when receiving a label like "messaging-app (enforce)".

service: fix playback of ogg files from messaging-app

e7cd801

These files are detected as having a video/3gpp MIME type. Since we know that they are audio files, let's make a little hack.

mardy force-pushed the cutify branch from b09a390 to e7cd801 Compare June 2, 2021 19:39

UniversalSuperBox approved these changes Jun 4, 2021

View reviewed changes

UniversalSuperBox merged commit f5ab42f into ubports:xenial Jun 4, 2021

OTA-18 automation moved this from In progress to QA Jun 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cutify #28

Cutify #28

mardy commented May 7, 2021 •

edited

lduboeuf commented May 28, 2021

lduboeuf commented May 29, 2021

mardy commented May 29, 2021

lduboeuf commented May 31, 2021

mardy commented May 31, 2021

lduboeuf commented May 31, 2021

lduboeuf commented May 31, 2021 •

edited

mardy commented Jun 2, 2021

lduboeuf commented Jun 2, 2021

mardy commented Jun 2, 2021

mardy commented Jun 2, 2021

lduboeuf commented Jun 2, 2021

UniversalSuperBox left a comment

Cutify #28

Cutify #28

Conversation

mardy commented May 7, 2021 • edited

lduboeuf commented May 28, 2021

lduboeuf commented May 29, 2021

mardy commented May 29, 2021

lduboeuf commented May 31, 2021

mardy commented May 31, 2021

lduboeuf commented May 31, 2021

lduboeuf commented May 31, 2021 • edited

mardy commented Jun 2, 2021

lduboeuf commented Jun 2, 2021

mardy commented Jun 2, 2021

mardy commented Jun 2, 2021

lduboeuf commented Jun 2, 2021

UniversalSuperBox left a comment

Choose a reason for hiding this comment

mardy commented May 7, 2021 •

edited

lduboeuf commented May 31, 2021 •

edited