Bug 134418 – [Win] Implement parts of the AVFOUNDATION_LOADER_DELEGATE logic for Windows

• Component: Media
• Status: RESOLVED FIXED

[Brent Fulgham]

This patch lands a partial implementation of the AVFOUNDATION_LOADER_DELEGATE logic in preparation for additional changes to come. This code builds, but is not exercised on Windows yet.

[Brent Fulgham]

Created attachment 234026 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/17491021>

[Brent Fulgham]

Created attachment 234029 [details]
Patch

[WebKit Commit Bot]

Attachment 234029 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:45:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/cf/CDMSessionAVFoundationCF.cpp:43:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
Total errors found: 3 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

(In reply to comment #4)
> Attachment 234029 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
> ERROR: Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:45:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
> ERROR: Source/WebCore/platform/graphics/avfoundation/cf/CDMSessionAVFoundationCF.cpp:43:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
> Total errors found: 3 in 20 files

These are necessary violations of style, and are explained in the comments in the source file.

[Eric Carlson]

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

> Source/WebCore/Modules/encryptedmedia/CDMPrivateMediaPlayer.cpp:-27
> -#import "config.h"
> -#import "CDMPrivateMediaPlayer.h"

Oops :-O

> Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:153
> +    RetainPtr<AVCFAssetResourceLoadingRequestRef> takeRequestForKeyURI(const String&);

Should this be a PassRefPtr<>?

> Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:927
> +#ifndef NDEBUG
> +    RetainPtr<CFURLRequestRef> urlRequest = AVCFAssetResourceLoadingRequestGetURLRequest(avRequest);
> +    RetainPtr<CFURLRef> requestURL = CFURLRequestGetURL(urlRequest.get());
> +    RetainPtr<CFStringRef> schemeRef = adoptCF(CFURLCopyScheme(requestURL.get()));
> +
> +    String scheme = schemeRef.get();
> +#endif

Did you mean to LOG() the scheme? If not, what does this code do?

> Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:1799
> +    String keyURI = keyURIRef.get();

This is only used inside of #if ENABLE(ENCRYPTED_MEDIA_V2) so it will fail in a release build if ENABLE_ ENCRYPTED_MEDIA_V2 isn't defined.

> Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.h:2
> + * Copyright (C) 2011, 2013, 2014 Apple Inc. All rights reserved.

Nit: 2013-2014.

[Brent Fulgham]

Created attachment 234030 [details]
Patch

[WebKit Commit Bot]

Attachment 234030 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/cf/WebCoreAVCFResourceLoader.cpp:45:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/cf/CDMSessionAVFoundationCF.cpp:43:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
Total errors found: 3 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Committed r170562: <http://trac.webkit.org/changeset/170562>