Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and fix metadata tracking api. JB#58070 #15

Merged
merged 8 commits into from Jan 12, 2023

Conversation

pvuorela
Copy link
Contributor

From the main commit:

Since the MprisController metaData instance was changing on different
apps to follow, qml code such as

MprisController {
  metaData.onTitleChanged: console.log(metadata.title)
}

...didn't work. The connection was created for the initial metaData
instance, not updated later on.

Fixed by a separate proxy class, keeping the returned instance of
MprisController always the same. The result is a bit verbose at the
moment but it's simple. On longer run we should probably better separate
the metadata instances on media and controller apps.

In addition a bunch of cleanup commits. Most notable being the one changing parent() calls to more explicit names. Was getting confused when sometimes parent was referring to qobject of the public class, or the public class owner of a private data, or private data of some other class etc.

parent() is conventionally the owning QObject instance in Qt.
Here it being used to that but also other needs was making it hard
to follow the code. Was often mistaking references to private as
references to owning object of the public instance.

And then there were strange setups like parent()->parent()->something(),
where the first parent was reference to another class instance private
and latter parent() to its public class.

Mostly mechanic changes except MprisPlayerAdaptor::minimumRate()
wasn't following the pattern for some reason.
"Connected" sounds like a boolean.
Same name as in the similar structure on the following else {} branch.
The metadata object doesn't change here and it's not expected to.
Since the MprisController metaData instance was changing on different
apps to follow, qml code such as

MprisController {
  metaData.onTitleChanged: console.log(metadata.title)
}

...didn't work. The connection was created for the initial metaData
instance, not updated later on.

Fixed by a separate proxy class, keeping the returned instance of
MprisController always the same. The result is a bit verbose at the
moment but it's simple. On longer run we should probably better separate
the metadata instances on media and controller apps.
src/mprisplayer.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Tomin1 Tomin1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@pvuorela pvuorela merged commit 16928b5 into master Jan 12, 2023
@Tomin1 Tomin1 deleted the fix_metadata_tracking_api branch March 31, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants