Bug 159250

Summary:

Adopt MediaRemote.

Product:

WebKit

Reporter:

Jer Noble <jer.noble>

Component:

New Bugs

Assignee:

Jer Noble <jer.noble>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric.carlson, hs85.jeong, jeremyj-wk, jonlee, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
WIP	none
WIP	none
Patch	none
Patch	eric.carlson: review+
Patch for landing	none
Patch for landing	none

Jer Noble

Reported 2016-06-29 00:06:55 PDT

Adopt MediaRemote.

Attachments
WIP (59.98 KB, patch) 2016-06-29 00:32 PDT, Jer Noble	no flags	Details Formatted Diff Diff
WIP (62.98 KB, patch) 2016-06-29 08:19 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch (62.03 KB, patch) 2016-06-29 08:49 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch (61.70 KB, patch) 2016-06-29 08:51 PDT, Jer Noble	eric.carlson: review+	Details Formatted Diff Diff
Patch for landing (63.23 KB, patch) 2016-06-29 09:34 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch for landing (61.72 KB, patch) 2016-06-29 09:36 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Jer Noble

Comment 1 2016-06-29 00:32:39 PDT

Created attachment 282329 [details] WIP

Eric Carlson

Comment 2 2016-06-29 08:12:39 PDT

Comment on attachment 282329 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=282329&action=review > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1589 > - m_buffered->add(presentationTimestamp.toDouble(), (presentationTimestamp + frameDuration + microsecond).toDouble()); > + m_buffered->add(presentationTimestamp.toDouble(), (presentationTimestamp + frameDuration).toDouble()); Is this related? > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm:2 > + * Copyright (C) 2014-2015 Apple Inc. All rights reserved. Nit: 2014-2015 -> 2016. > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm:156 > + if (std::isfinite(duration) && duration != MediaPlayer::invalidTime()) > + CFDictionarySetValue(info.get(), kMRMediaRemoteNowPlayingInfoDuration, cfDuration.get()); Nit: you don't need to create the CFNumber unless it will be added to the info dictionary. > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm:163 > + CFDictionarySetValue(info.get(), kMRMediaRemoteNowPlayingInfoElapsedTime, cfCurrentTime.get()); Ditto. > Source/WebCore/platform/mac/RemoteCommandListenerMac.mm:60 > + auto commandInfoArray = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, sizeof(supportedCommands) / sizeof(MRMediaRemoteCommand), &kCFTypeArrayCallBacks)); You can use ARRAY_SIZE here.

Jer Noble

Comment 3 2016-06-29 08:19:31 PDT

Created attachment 282347 [details] WIP

Eric Carlson

Comment 4 2016-06-29 08:24:36 PDT

Comment on attachment 282347 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=282347&action=review > Source/WebCore/platform/mac/RemoteCommandListenerMac.h:27 > +#ifndef RemoteCommandListenerMac_h > +#define RemoteCommandListenerMac_h Nit: this can use "#pragma once"

Jer Noble

Comment 5 2016-06-29 08:49:40 PDT

(In reply to comment #2) > Comment on attachment 282329 [details] > WIP > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282329&action=review > > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1589 > > - m_buffered->add(presentationTimestamp.toDouble(), (presentationTimestamp + frameDuration + microsecond).toDouble()); > > + m_buffered->add(presentationTimestamp.toDouble(), (presentationTimestamp + frameDuration).toDouble()); > > Is this related? Whoops. Nope. > > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm:2 > > + * Copyright (C) 2014-2015 Apple Inc. All rights reserved. > > Nit: 2014-2015 -> 2016. Ok. > > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm:156 > > + if (std::isfinite(duration) && duration != MediaPlayer::invalidTime()) > > + CFDictionarySetValue(info.get(), kMRMediaRemoteNowPlayingInfoDuration, cfDuration.get()); > > Nit: you don't need to create the CFNumber unless it will be added to the > info dictionary. > > > Source/WebCore/platform/audio/mac/MediaSessionManagerMac.mm:163 > > + CFDictionarySetValue(info.get(), kMRMediaRemoteNowPlayingInfoElapsedTime, cfCurrentTime.get()); > > Ditto. > > > Source/WebCore/platform/mac/RemoteCommandListenerMac.mm:60 > > + auto commandInfoArray = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, sizeof(supportedCommands) / sizeof(MRMediaRemoteCommand), &kCFTypeArrayCallBacks)); > > You can use ARRAY_SIZE here. Ok. (In reply to comment #4) > Comment on attachment 282347 [details] > WIP > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282347&action=review > > > Source/WebCore/platform/mac/RemoteCommandListenerMac.h:27 > > +#ifndef RemoteCommandListenerMac_h > > +#define RemoteCommandListenerMac_h > > Nit: this can use "#pragma once" Ok.

Jer Noble

Comment 6 2016-06-29 08:49:51 PDT

Created attachment 282348 [details] Patch

Jer Noble

Comment 7 2016-06-29 08:51:36 PDT

Created attachment 282349 [details] Patch

Jer Noble

Comment 8 2016-06-29 09:34:12 PDT

Created attachment 282351 [details] Patch for landing

Jer Noble

Comment 9 2016-06-29 09:36:43 PDT

Created attachment 282353 [details] Patch for landing