<a class="header-button" href="https://bugzilla.mozilla.org/home" title="Go to home page"> Bugzilla

Assignee

Comment 2

•

4 years ago

It looks like media control related, not sure if new OS adds new API or supports some old API usage to show that on their new notification center.

Blocks: media-control

Assignee

Updated

•

4 years ago

Assignee: nobody → alwu

Assignee

Comment 3

•

4 years ago

Attached file Bug 1671626 - part1 : use weak link to get needed classes. (obsolete) — Details

Assignee

Comment 4

•

4 years ago

Attached file Bug 1671626 - part2 : set metadata on nowPlayingInfo. (obsolete) — Details

Depends on D97066

Assignee

Comment 5

•

4 years ago

Attached file Bug 1671626 - part1 : remove `MediaPlayerWrapper`. — Details

Updated

•

4 years ago

Attachment #9187817 - Attachment is obsolete: true

Updated

•

4 years ago

Attachment #9187818 - Attachment is obsolete: true

Assignee

Comment 6

•

4 years ago

Attached file Bug 1671626 - part2 : set metadata on nowPlayingInfo. (obsolete) — Details

Depends on D97223

Assignee

Comment 7

•

4 years ago

I've finished the implementation, but now got stuck on the build issue, because the machine on the try server is still using 10.11 SDK where MediaPlayer hasn't been introduced yet.

[2] https://treeherder.mozilla.org/logviewer?job_id=322229528&repo=try&lineNumber=29394

Assignee

Comment 8

•

4 years ago

•

Edited

I'm still suffering from the build/link issues, here I'm going to describe the problems I encountered.

Use MediaPlayer framework directly (failed)
As we are still using 10.11 on the try server, which is an older SDK, we are not able to find MediaPlayer framework to link on that SDK version.
Declare extern ... MPMediaItemPropertyXXX (failed)
Then I tried to define something following in MediaPlayerWrapper.h

extern NSString* const _Nullable MPMediaItemPropertyTitle;
extern NSString* const _Nullable MPMediaItemPropertyArtist;
extern NSString* const _Nullable MPMediaItemPropertyAlbumTitle;

On the linking stage, it would show the error of not being able to find those symbols. Those variables are originally defined in MPMediaItem.h, which look like that,

MP_EXTERN NSString * const MPMediaItemPropertyTitle                                                                     // filterable
    MP_API(ios(3.0), tvos(9.0), watchos(5.0), macos(10.12.2));
@property (nonatomic, readonly, nullable) NSString *title MP_API(ios(7.0));

MP_EXTERN NSString * const MPMediaItemPropertyAlbumTitle
    MP_API(ios(3.0), tvos(9.0), watchos(5.0), macos(10.12.2));                                                          // filterable
@property (nonatomic, readonly, nullable) NSString *albumTitle MP_API(ios(7.0));

MP_EXTERN NSString * const MPMediaItemPropertyArtist                                                                    // filterable
    MP_API(ios(3.0), tvos(9.0), watchos(5.0), macos(10.12.2));
@property (nonatomic, readonly, nullable) NSString *artist MP_API(ios(7.0));

So they are also not defined there, because I think MP_EXTERN is a term similar with extern?

Declare MPMediaItemPropertyXXX with linking (failed)

So I was thinking about if that could be fixed by manually adding link option, such like [1], then I added following codes,
[1] https://searchfox.org/mozilla-central/rev/7b40f0b246ad0b54975b1525811f2ad599b95f33/toolkit/library/moz.build#75-79

LDFLAGS += ["-Wl,-U,_MPMediaItemPropertyTitle"]
LDFLAGS += ["-Wl,-U,_MPMediaItemPropertyArtist"]
LDFLAGS += ["-Wl,-U,_MPMediaItemPropertyAlbumTitle"]

The result is that, I could pass the build, however, it failed immediately when I ran Firefox, the error is that,

dlopen(/Users/alastor/mozilla-dev/mozilla-central/obj-debug-x86_64-apple-darwin20.1.0/dist/NightlyDebug.app/Contents/MacOS/XUL, 265): Symbol not found: _MPMediaItemPropertyAlbumTitle
  Referenced from: /Users/alastor/mozilla-dev/mozilla-central/obj-debug-x86_64-apple-darwin20.1.0/dist/NightlyDebug.app/Contents/MacOS/XUL
  Expected in: flat namespace
 in /Users/alastor/mozilla-dev/mozilla-central/obj-debug-x86_64-apple-darwin20.1.0/dist/NightlyDebug.app/Contents/MacOS/XUL
Couldn't load XPCOM.

import <MediaPlayer/MPMediaItem.h> on 12.2+ (failed)

Then I thought probably I can't simply override the declaration for things using MP_EXTERN, I tried to import the header file directly.

#if defined(MAC_OS_X_VERSION_10_12_2) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12_2
  #import <MediaPlayer/MPMediaItem.h>
#endif

That allows me to pass build, and I could also start my Firefox without issue. However, when running to the code where first access to those MPMediaItemPropertyXXX, the problem would crash immediately with EXC_BAD_ACCESS. I used lldb to check if they are still dangling, but I can see they all have value, so I don't really understand this error.

(lldb) p MPMediaItemPropertyTitle
warning: `this' is not accessible (substituting 0)
(__NSCFConstantString *) $0 = 0x00007fff8277d3e8 "title"
(lldb) p MPMediaItemPropertyArtist
warning: `this' is not accessible (substituting 0)
(__NSCFConstantString *) $1 = 0x00007fff827802e8 "artist"
(lldb) p MPMediaItemPropertyAlbumTitle
warning: `this' is not accessible (substituting 0)
(__NSCFConstantString *) $2 = 0x00007fff82781ea8 "albumTitle"

Declare and define MPMediaItemPropertyXXX (failed only on the try server gTest)

Here I went to my last option, force to redefine them manually in MediaPlayerWrapper.h

#define MPMediaItemPropertyTitle @"title"
#define MPMediaItemPropertyArtist @"artist"
#define MPMediaItemPropertyAlbumTitle @"albumTitle"

Okay, this method allows me to build and run Firefox successfully on both the 10.12+ and the 10.11 machine on the try server. However, it would make my gTest fail on the 10.14 machine on the try server. I can successfully pass my gTest on my local environment, but on the 10.14 machine on the try server, it seems that I can't set the value to MPNowPlayingInfoCenter's nowPlayingInfo....

Here is my implementation, which works well for me on Big Sur. But on the 10.14 machine on the try server, I can see the results in DD are correct, which means I set the value correctly into the local variable nowPlayingInfo. However, the results in DD1 are all empty, which means MPNowPlayingInfoCenter didn't update its nowPlayingInfo. [2]

void MediaHardwareKeysEventSourceMacMediaCenter::SetMediaMetadata(
    const dom::MediaMetadataBase& aMetadata) {
  NSMutableDictionary* nowPlayingInfo = [NSMutableDictionary dictionary];
  [nowPlayingInfo setObject:nsCocoaUtils::ToNSString(aMetadata.mTitle)
                     forKey:MPMediaItemPropertyTitle];
  [nowPlayingInfo setObject:nsCocoaUtils::ToNSString(aMetadata.mArtist)
                     forKey:MPMediaItemPropertyArtist];
  [nowPlayingInfo setObject:nsCocoaUtils::ToNSString(aMetadata.mAlbum)
                     forKey:MPMediaItemPropertyAlbumTitle];
  NSLog(@"DD | Title in=%s out=%@", NS_ConvertUTF16toUTF8(aMetadata.mTitle).get(),
        nowPlayingInfo[MPMediaItemPropertyTitle]);
  NSLog(@"DD | Artist=%@", nowPlayingInfo[MPMediaItemPropertyArtist]);
  NSLog(@"DD | Album=%@", nowPlayingInfo[MPMediaItemPropertyAlbumTitle]);
  MPNowPlayingInfoCenter* center =
      (MPNowPlayingInfoCenter*)[mpNowPlayingInfoCenterClass defaultCenter];
  center.nowPlayingInfo = nowPlayingInfo;
  NSLog(@"DD1 | Title=%@", center.nowPlayingInfo[MPMediaItemPropertyTitle]);
  NSLog(@"DD1 | Artist=%@", center.nowPlayingInfo[MPMediaItemPropertyArtist]);
  NSLog(@"DD1 | Album=%@", center.nowPlayingInfo[MPMediaItemPropertyAlbumTitle]);
}

Assignee

Updated

•

4 years ago

Component: Audio/Video → Widget: Cocoa

Assignee

Comment 9

•

4 years ago

We just upgraded the SDK to 10.12 on the try server (finally!) \o/ So I'm going to test my patches again to see if they work on the try server.

Assignee

Comment 10

•

4 years ago

Since upgrading to 10.12, now my patches can build successfully on the try server. However, I still encouter same issue (no.5) as I described in comment8 where I would fail on the gtest on the try server.

I can run the ability of showing metadata and pass the gTest on my local enviroment successfully, but it keeps failling on the try server. From the [1] log result, it seems that I were not able to set nowPlayingInfo on the default MPNowPlayingInfoCenter, but I have no idea why....

In other test cases, we indeed change MPNowPlayingInfoCenter's playback, so I don't understand why we would fail to set nowPlayingInfo.

Hi, Stephen,
I wonder if you have any thought or experience about this kind of error which can help me solve this mysterious problem.
Thank you so much.

Flags: needinfo?(spohl.mozilla.bugs)

Comment 11

•

4 years ago

This does seem strange indeed. Can you run this with some more logging? For example, could you try to print the entire nowPlayingInfo dictionary via NSLog() using the %@ specifier? Possibly before and after you're trying to set it?

Flags: needinfo?(spohl.mozilla.bugs)

Assignee

Comment 12

•

4 years ago

Yes, I've printed them and here is the result. The patch contains delog log is here.

[task 2020-12-09T22:09:39.174Z] 22:09:39     INFO -  2020-12-09 22:09:39.172 firefox[1677:10768] DD | Title in=MediaPlayback out=MediaPlayback
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | Artist=Firefox
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | Album=Mozilla
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | MPMediaItemPropertyTitle=title
[task 2020-12-09T22:09:39.193Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | MPMediaItemPropertyArtist=artist
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD | MPMediaItemPropertyAlbumTitle=albumTitle
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD1 | Title=
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD1 | Artist=
[task 2020-12-09T22:09:39.194Z] 22:09:39     INFO -  2020-12-09 22:09:39.173 firefox[1677:10768] DD1 | Album=

From above log, you can see that nowPlayingInfo in defaultCenter wasn't updated. But that didn't happen on running either Firefox or gTest on my local 11.0.1 enviorment, which makes me super confused.

Updated

•

4 years ago

Flags: needinfo?(alwu)

Comment 14

•

4 years ago

I don't know why my previous comment was marked as private. Submitting it again in public:

Could you let me know how I can run your latest patches (and this gtest) on try myself? What exactly would I need to push?

I'm wondering if there is a race somewhere with the newly introduced center.nowPlayingInfo = nil; in MediaHardwareKeysEventSourceMacMediaCenter::EndListeningForEvents in https://phabricator.services.mozilla.com/D97224. The native APIs are asynchronous to a certain extent.

Assignee

Comment 15

•

4 years ago

If you want to run it locally, simply apply those two patches and run ./mach gtest MediaHardwareKeysEventSourceMacMediaCenter.\*.

When running gTest on the try server, I use ./mach try chooser and choose gTest under Test Suites on macOS, then gTest should be able to run on the try server. I just pushed two tasks on the try, one for the orignal patch with debug log, and another one is to delete the center.nowPlayingInfo = nil in EndListeningForEvents().

However, EndListeningForEvents() would be called when (1) close the source (2) destroy the source, which are both impossible to happen when we check the assertion, because at the time we check the assertion, the source is still open. So that failure is really myterious to me.

Yes in the beginning I was also considering if those API are actuaaly async, so maybe that is why I failed on checking the nowPlayingInfo immediately after I set its value. However, after I added the debug log, I did see the nowPlayingInfo in the defaultCenterchanged immediately after I set its vaule by checking the printed log.

Thank you so much!

Flags: needinfo?(alwu) → needinfo?(spohl.mozilla.bugs)

Assignee

Comment 16

•

4 years ago

If we really can't find the solution for that, I would probably to remove the test :( but that would be the last choice.

My try push: https://treeherder.mozilla.org/jobs?repo=try&revision=11a711d655d58d826c5c115ba2fd9fdc8888d2ca&selectedTaskRun=FGVP9KxWRH2MEnFi8aeMaA.0

Comment 17

•

4 years ago

I have been looking into this today and I suspect that the SDK version on try is not sufficiently high to have built-in support for the MediaPlayer library. I am not sure how your build on try succeeded, since my attempts fail with this error:

ld: framework not found MediaPlayer
See: https://treeherder.mozilla.org/logviewer?job_id=324747396&repo=try&lineNumber=47208

This library is expected to be supported in macOS 10.12.1+. I have asked about the specific SDK version that is now on try in bug 1680152 comment 12.

Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(alwu)

Assignee

Comment 18

•

4 years ago

•

Edited

That is weird, if you check my push in comment 15, then they were both using 10.12 SDK for building.

Considering the current tests we have, I think we still have the robust test coverage even if we don't provide gtest for this. I would like to seperate this implementation and gtest (filed bug1683020), and land the implementation first.

Flags: needinfo?(alwu)

[1]
https://treeherder.mozilla.org/logviewer?job_id=324776746&repo=try&lineNumber=34512
https://treeherder.mozilla.org/logviewer?job_id=324768209&repo=try&lineNumber=1184
https://treeherder.mozilla.org/logviewer?job_id=324749219&repo=try&lineNumber=740

Assignee

Comment 19

•

4 years ago

I randomly chose three tasks [1] from the try server, and they all used 10.12 SDK, so I'm not sure why your task would build with 10.11 SDk.

Assignee

Updated

•

4 years ago

Blocks: 1683020

Assignee

Comment 20

•

4 years ago

Attached file Bug 1671626 - part2 : set metadata on nowPlayingInfo. — Details

Depends on D97223

Updated

•

4 years ago

Attachment #9188161 - Attachment description: Bug 1671626 - part1 : use weak link to get needed classes. → Bug 1671626 - part1 : remove `MediaPlayerWrapper`.

Alexandru Michis [:malexandru]

Updated

•

4 years ago

Attachment #9188163 - Attachment is obsolete: true

Pulsebot

Comment 21

•

4 years ago

Pushed by alwu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ce98ac27b80 part1 : remove `MediaPlayerWrapper`. r=spohl https://hg.mozilla.org/integration/autoland/rev/79cb66d1fe96 part2 : set metadata on nowPlayingInfo. r=spohl

Comment 22

•

4 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/5ce98ac27b80
https://hg.mozilla.org/mozilla-central/rev/79cb66d1fe96

Status: NEW → RESOLVED

Closed: 4 years ago

status-firefox86: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 86 Branch

Ryan VanderMeulen [:RyanVM]

Updated

•

4 years ago

status-firefox82: affected → wontfix

status-firefox83: affected → wontfix

Ciprian Georgiu, Desktop QA

Updated

•

4 years ago

Flags: qe-verify+

Daniel Bodea [:danibodea]

Comment 23

•

4 years ago

I can confirm this fix on Mac OS 11.1 with Nightly v87.0a1 from 2021-01-26 (also from 2021-01-29), but not on Beta v86.0b1 or v86.0b3.
The play icon from the top bar does not appear as it does for nightly builds. Is it supposed to?

status-firefox87: --- → verified

Flags: qe-verify+ → needinfo?(alwu)