Bug 180542 - [EME] support update() for FairPlayStreaming in Modern EME API
Summary: [EME] support update() for FairPlayStreaming in Modern EME API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-07 13:05 PST by Jer Noble
Modified: 2017-12-11 23:21 PST (History)
5 users (show)

See Also:


Attachments
Patch (14.49 KB, patch)
2017-12-07 13:31 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (15.21 KB, patch)
2017-12-07 14:13 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (15.23 KB, patch)
2017-12-07 15:21 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2017-12-07 13:05:30 PST
[EME] support update() for FairPlayStreaming in Modern EME API
Comment 1 Jer Noble 2017-12-07 13:31:28 PST
Created attachment 328730 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2017-12-07 13:31:29 PST
<rdar://problem/35917563>
Comment 3 Eric Carlson 2017-12-07 13:43:43 PST
Comment on attachment 328730 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328730&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:968
> +    for (auto& sourceBuffer : m_mediaSourcePrivate->sourceBuffers())
> +        sourceBuffer->setCDMInstance(&instance);
> +
> +    m_cdmInstance = &instance;

Nit: would it be safer to set m_cdmInstance first in case the source buffer callbacks have side effects?
Comment 4 Jer Noble 2017-12-07 14:13:43 PST
Created attachment 328739 [details]
Patch for landing
Comment 5 Jer Noble 2017-12-07 15:21:56 PST
Created attachment 328749 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-12-11 17:17:25 PST
Comment on attachment 328749 [details]
Patch for landing

Clearing flags on attachment: 328749

Committed r225766: <https://trac.webkit.org/changeset/225766>
Comment 7 WebKit Commit Bot 2017-12-11 17:17:26 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Joseph Pecoraro 2017-12-11 22:41:15 PST
This appears to have broken the High Sierra build:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20%28Build%29/builds/2072/steps/compile-webkit/logs/errors

> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:770:71: error: cannot initialize a parameter of type 'id<AVContentKeyRecipient> _Nonnull' with an rvalue of type 'PtrType' (aka 'AVStreamDataParser *')

It looks like the AVFoundationSPI definition of AVStreamDataParser wasn't set up to conform to the AVContentKeyRecipient @protocol.

This should be fixed with something like:

    + #import <AVFoundation/AVContentKeySession.h>
    ...
    + #if GUARD
    + @interface AVStreamDataParser () <AVContentKeyRecipient>
    + @end
    + #endif

But since AVContentKeyRecipient was only conditionally available:

    API_AVAILABLE(macos(10.12.4), ios(10.3), tvos(10.2), watchos(3.3))
    @protocol AVContentKeyRecipient
    ...
    @end

There will likely need to be some version GUARD above that makes sense.
Comment 9 Joseph Pecoraro 2017-12-11 22:57:08 PST
> But since AVContentKeyRecipient was only conditionally available:
> 
>     API_AVAILABLE(macos(10.12.4), ios(10.3), tvos(10.2), watchos(3.3))
>     @protocol AVContentKeyRecipient
>     ...
>     @end
> 
> There will likely need to be some version GUARD above that makes sense.

I guess HAVE(AVCONTENTKEYSESSION) might be an appropriate guard:

    #if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300)
    #define HAVE_AVCONTENTKEYSESSION 1
    #endif
Comment 10 Joseph Pecoraro 2017-12-11 23:21:46 PST
I attempted a build fix:
<https://trac.webkit.org/r225777>

Lets see what the bots do!