Bug 403289

Summary:	MediaPlayer1 interface is NULL after changing audio player on BT source
Product:	[Frameworks and Libraries] frameworks-bluez-qt	Reporter:	snehal.tan
Component:	general	Assignee:	David Rosca <nowrep>
Status:	RESOLVED FIXED
Severity:	normal	CC:	nate
Priority:	NOR
Version First Reported In:	5.54.0
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://commits.kde.org/bluez-qt/9197a45652be65a001807f9c163a5d005fb8c98b	Version Fixed/Implemented In:	5.57
Sentry Crash Report:

Description snehal.tan 2019-01-17 01:31:47 UTC

Setup: I am using bluez-qt package in isolation to develop a bluetooth a2dp sink application. I am not using any other packages from kde frameworks. I can reproduce the bug on my system, but I do not have the setup to reproduce the bug on a KDE platform. Below are the steps I THINK should trigger the bug. Please note that only audio sink application using bluez-qt API will trigger this bug.

STEPS TO REPRODUCE
1. Establish the bluetooth connection between audio source (e.g. smartphone) and audio sink (e.g. PC can be configured as sink).
2. Start a audio playback on smartphone and audio should start playing on the sink. MediaPlayer name (typically the app playing the audio on smartphone) and track information will be visible on the audio sink application. This will be done through BluezQt::MediaPlayer signals.
3. On source switch to a different audio playback application. Audio streaming will continue but MediaPlayer control and track information will not available on the audio sink application.

OBSERVED RESULT
Audio streaming will continue but MediaPlayer control and track information will not available on the audio sink application. This is because BluezQt::Devie::MediaPlayerPtr mediaplayer is NULL.

EXPECTED RESULT
BluezQt::Devie::MediaPlayerPtr mediaplayer should point to new interface created after switching the audio app on source.

ADDITIONAL INFORMATION
I have checkout the github repo and analysed the bug.
Object path .../PlayerX is not checked before removing MediaPlayer1 interface. BluezQt only saves most recently added object /playerX. But while
removing MediaPlayer1 interface object path is not checked which could
lead to accidental removal a valid interface. This can happen when
events 'interfacesAdded' and 'interfacesRemoved' arrive in different
order.
Consider scenario: player0:interfacesAdded -> player1:interfacesAdded ->
player0:interfacesRemoved. In this scenario MediaPlayer1 interface of
player1 will be removed if object path is not checked.

Comment 1 snehal.tan 2019-01-17 01:46:48 UTC

D-Bus log of the scenario described in the original comment. Please see the object paths and order in which the members InterfacesAdded and InterfacesRemoved are called.

signal time=946700748.013475 sender=:1.0 -> destination=(null destination) serial=296 path=/; interface=org.freedesktop.DBus.ObjectManager; member=InterfacesAdded
   object path "/org/bluez/hci0/dev_90_97_F3_F7_33_31/player0"
   array [
      dict entry(
         string "org.freedesktop.DBus.Introspectable"
         array [
         ]
      )
      dict entry(
         string "org.bluez.MediaPlayer1"
         array [
            dict entry(
               string "Position"
               variant                   uint32 0
            )
            dict entry(
               string "Device"
               variant                   object path "/org/bluez/hci0/dev_90_97_F3_F7_33_31"
            )
         ]
      )
      dict entry(
         string "org.freedesktop.DBus.Properties"
         array [
         ]
      )
   ]

signal time=946700769.777729 sender=:1.0 -> destination=(null destination) serial=331 path=/; interface=org.freedesktop.DBus.ObjectManager; member=InterfacesAdded
   object path "/org/bluez/hci0/dev_90_97_F3_F7_33_31/player2"
   array [
      dict entry(
         string "org.freedesktop.DBus.Introspectable"
         array [
         ]
      )
      dict entry(
         string "org.bluez.MediaPlayer1"
         array [
            dict entry(
               string "Position"
               variant                   uint32 0
            )
            dict entry(
               string "Device"
               variant                   object path "/org/bluez/hci0/dev_90_97_F3_F7_33_31"
            )
         ]
      )
      dict entry(
         string "org.freedesktop.DBus.Properties"
         array [
         ]
      )
   ]

signal time=946700769.821715 sender=:1.0 -> destination=(null destination) serial=333 path=/; interface=org.freedesktop.DBus.ObjectManager; member=InterfacesRemoved
   object path "/org/bluez/hci0/dev_90_97_F3_F7_33_31/player0"
   array [
      string "org.freedesktop.DBus.Properties"
      string "org.freedesktop.DBus.Introspectable"
      string "org.bluez.MediaFolder1"
      string "org.bluez.MediaPlayer1"
   ]

Comment 2 David Rosca 2019-01-17 12:43:51 UTC

Thanks for the detailed description!

Can you please test https://phabricator.kde.org/D18315 ?

Comment 3 snehal.tan 2019-01-17 21:02:41 UTC

(In reply to David Rosca from comment #2)
> Thanks for the detailed description!
> 
> Can you please test https://phabricator.kde.org/D18315 ?

I am running Qt5.6, so I could only test with v5.25. I cherrypicked the change on 5.25 and I can confirm it works for MediaPlaeyr1 interface. New PlayerX object can now be accessed correctly for all the properties.

Comment 4 David Rosca 2019-03-18 11:09:50 UTC

Git commit 9197a45652be65a001807f9c163a5d005fb8c98b by David Rosca.
Committed on 18/03/2019 at 10:09.
Pushed by drosca into branch 'master'.

Device: Check object path in interfaces removed slot

Only remove Input and MediaPlayer objects when path matches.
FIXED-IN: 5.57

Differential Revision: https://phabricator.kde.org/D18315

M  +2    -0    autotests/fakebluez/deviceinterface.h
M  +13   -0    autotests/fakebluez/devicemanager.cpp
M  +1    -0    autotests/fakebluez/devicemanager.h
M  +1    -0    autotests/fakebluez/object.h
M  +27   -0    autotests/mediaplayertest.cpp
M  +2    -0    autotests/mediaplayertest.h
M  +2    -3    src/device_p.cpp
M  +1    -2    src/input.cpp
M  +1    -0    src/input_p.h
M  +1    -0    src/mediaplayer_p.cpp
M  +1    -0    src/mediaplayer_p.h

https://commits.kde.org/bluez-qt/9197a45652be65a001807f9c163a5d005fb8c98b