Bug 133747 - [EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
Summary: [EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-11 10:35 PDT by Jer Noble
Modified: 2014-06-16 10:24 PDT (History)
5 users (show)

See Also:


Attachments
Patch (25.51 KB, patch)
2014-06-11 12:19 PDT, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2014-06-11 10:35:57 PDT
[EME][Mac] Propagate errors from AVSampleBufferDisplayLayer through to MSE
Comment 1 Jer Noble 2014-06-11 12:19:30 PDT
Created attachment 232885 [details]
Patch
Comment 2 Eric Carlson 2014-06-11 12:38:05 PDT
Comment on attachment 232885 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionMediaSourceAVFObjC.mm:68
> +{
> +    m_parent->unregisterForErrorNotifications(this);
>  }

Is it worth NULLing m_client here?
Comment 3 Jer Noble 2014-06-11 13:52:09 PDT
(In reply to comment #2)
> (From update of attachment 232885 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232885&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionMediaSourceAVFObjC.mm:68
> > +{
> > +    m_parent->unregisterForErrorNotifications(this);
> >  }
> 
> Is it worth NULLing m_client here?

Probably not, since the object itself is about to get destroyed.
Comment 4 Jer Noble 2014-06-11 14:02:55 PDT
Committed r169830: <http://trac.webkit.org/changeset/169830>
Comment 5 Darin Adler 2014-06-16 10:24:41 PDT
Comment on attachment 232885 [details]
Patch

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

> Source/WebCore/Modules/encryptedmedia/MediaKeySession.h:48
> +class MediaKeySession final : public RefCounted<MediaKeySession>, public EventTargetWithInlineData, public ActiveDOMObject, public CDMSessionClient {

Why public instead of private inheritance?

> Source/WebCore/Modules/encryptedmedia/MediaKeySession.h:90
> +    virtual void sendMessage(Uint8Array*, String destinationURL);
> +    virtual void sendError(MediaKeyErrorCode, unsigned long systemCode);

Why protected instead of private? Why not marked override?

> Source/WebCore/platform/graphics/SourceBufferPrivateClient.h:87
> +    virtual void sourceBufferPrivateDidReceiveRenderingError(SourceBufferPrivate*, int errocCode) = 0;

Typo here: "errocCode".

> Source/WebCore/platform/graphics/avfoundation/objc/CDMSessionMediaSourceAVFObjC.h:38
> +class CDMSessionMediaSourceAVFObjC : public CDMSession, public SourceBufferPrivateAVFObjCErrorClient {

Why public instead of private inheritance?

> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:828
> +    int errorCode = [[[error userInfo] valueForKey:@"OSStatus"] intValue];

Seems like this should be inside the "if (m_client)".