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

Update play queue metadata #2453

Merged
merged 4 commits into from
Jul 21, 2019
Merged

Update play queue metadata #2453

merged 4 commits into from
Jul 21, 2019

Conversation

m0n1ker
Copy link
Contributor

@m0n1ker m0n1ker commented Jul 10, 2019

This PR includes

  • Bug fix for the duration metadata field. item.getDuration() returns the duration in seconds, while the metadata object expects this to be in milliseconds.
  • Add the current track number and total number of tracks in the playlist to the metadata. Some Bluetooth players prefer this information be present.

These metadata changes have been tested using a Pioneer car stereo over Bluetooth.

Fixes #2240

@m0n1ker m0n1ker changed the title Update play queue metatada Update play queue metadata Jul 10, 2019
@Stypox
Copy link
Member

Stypox commented Jul 10, 2019

Could this pr have anything to do with #1467 and #228? By reading the lines you edited it seems like those issues can be closed (after this pr is merged).

EDIT: it seems like the video title is missing from the metadata, is that true? If so, could you please add it?

@Redirion
Copy link
Member

Redirion commented Jul 10, 2019

@Stypox the title and mediaId are set in the constructor a few lines above.
Sometimes its just a device issue.

I was first excited when I saw this PR as I was the one that introduced the "additionalMetadata" bundle but getDuration didn't work for me.
But I guess it is also a device issue for me as my car (Kia Stinger 2018) does still not show the correct duration even with these changes added.

But I am happy that it works for you @m0n1ker
I am just unable to report an improvement for me personally but at least there is no regression :)

edit: maybe we should still put METADATA_KEY_TITLE in case this field is read by some awkward bluetooth devices?

@m0n1ker
Copy link
Contributor Author

m0n1ker commented Jul 11, 2019

@Stypox This might help #1467, but I kind of doubt it. I'm running android 9 on a Galaxy S9 and it does not show the media controls as described in the issue. I only see the notification widget, which is working for me, so I'm not able to test this specific issue on my device.

As far as #228, again I think it will make the exposed metadata more reliable but I'm not sure this really resolves the feature request as stated. I looked at adding the album art to the metadata, but it was causing some other issues that I haven't been able to solve. If I can get that working I can submit another PR for it.

@Redirion I'm surprised this didn't change anything in your car. It's new enough it should be able to correctly handle metadata, but bluetooth receivers are weird sometimes. I've verified the metadata being sent with the bluetooth-player utility that is a part of Linux's "bluez" bluetooth stack. It prints out metadata and playback status messages as they are sent, and I'm seeing the proper data and playback states being sent now.

I agree that adding METADATA_KEY_TITLE to the additional metadata would be a good idea. It doesn't hurt anything, and if it makes it more widely compatible with bluetooth receivers that's an easy win. I'll push another commit after this comment.

@m0n1ker
Copy link
Contributor Author

m0n1ker commented Jul 11, 2019

As an additional note, I think the issue with #228 has more to do with the fact this app is implementing MediaDescriptionCompat but not MediaMetadataCompat. From what I can tell, the description object is meant for lightweight display-only purposes such as showing items in the play queue. The full MediaMetadataCompat object is the preferred one for showing details about an individual item in the queue, such as the currently playing track, and adding support for this may actually help both the bluetooth car audio issues and the issues associated with #228. This is something I'd like to explore more when I have some time.

@TobiGr
Copy link
Contributor

TobiGr commented Jul 13, 2019

@m0n1ker Thanks! The code looks good.

As an additional note, I think the issue with #228 has more to do with the fact this app is implementing MediaDescriptionCompat but not MediaMetadataCompat. From what I can tell, the description object is meant for lightweight display-only purposes such as showing items in the play queue. The full MediaMetadataCompat object is the preferred one for showing details about an individual item in the queue, such as the currently playing track, and adding support for this may actually help both the bluetooth car audio issues and the issues associated with #228. This is something I'd like to explore more when I have some time.

Yes, that could solve the problems. I am glad that you want to take a look at it.

Is someone capable of testing this with a different car or device? I don't own a car which supports Bluetooth. If not, this shouldn't be a problem, because the changes don't look like they can break something.

@theScrabi theScrabi added the bug Issue is related to a bug label Jul 21, 2019
@theScrabi theScrabi merged commit 6787d02 into TeamNewPipe:dev Jul 21, 2019
This was referenced Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Playing title's current time and length is not displayed on car bluetooth player
5 participants