Bug 134418 - [Win] Implement parts of the AVFOUNDATION_LOADER_DELEGATE logic for Windows
Summary: [Win] Implement parts of the AVFOUNDATION_LOADER_DELEGATE logic for Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks: 137710
  Show dependency treegraph
 
Reported: 2014-06-27 15:26 PDT by Brent Fulgham
Modified: 2014-10-21 11:56 PDT (History)
12 users (show)

See Also:


Attachments
Patch (65.49 KB, patch)
2014-06-27 15:33 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (83.53 KB, patch)
2014-06-27 15:44 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (83.02 KB, patch)
2014-06-27 16:15 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-06-27 15:26:37 PDT
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.
Comment 1 Brent Fulgham 2014-06-27 15:33:46 PDT
Created attachment 234026 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2014-06-27 15:34:23 PDT
<rdar://problem/17491021>
Comment 3 Brent Fulgham 2014-06-27 15:44:40 PDT
Created attachment 234029 [details]
Patch
Comment 4 WebKit Commit Bot 2014-06-27 15:46:13 PDT
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.
Comment 5 Brent Fulgham 2014-06-27 15:51:55 PDT
(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.
Comment 6 Eric Carlson 2014-06-27 15:59:04 PDT
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.
Comment 7 Brent Fulgham 2014-06-27 16:15:00 PDT
Created attachment 234030 [details]
Patch
Comment 8 WebKit Commit Bot 2014-06-27 16:17:06 PDT
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.
Comment 9 Brent Fulgham 2014-06-27 17:20:10 PDT
Committed r170562: <http://trac.webkit.org/changeset/170562>