Bug 122043

Summary: [Mac] Add AudioTrack support to MediaPlayerPrivateAVFObC.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, esprehn+autocc, glenn, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 122049    
Attachments:
Description Flags
Work in progress
none
Patch eric.carlson: review+

Description Jer Noble 2013-09-27 17:13:32 PDT
[Mac] Add AudioTrack support to MediaPlayerPrivateAVFObC.
Comment 1 Radar WebKit Bug Importer 2013-09-27 17:14:28 PDT
<rdar://problem/15103997>
Comment 2 Jer Noble 2013-09-27 17:16:20 PDT
Created attachment 212858 [details]
Work in progress
Comment 3 Jer Noble 2013-09-27 17:32:30 PDT
Created attachment 212859 [details]
Patch
Comment 4 Eric Carlson 2013-09-28 13:27:12 PDT
Comment on attachment 212859 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/AudioTrackPrivateAVFObjC.mm:93
> +    for (AVMetadataItem* item in [assetTrack commonMetadata]) {
> +        if ([item.key isEqual:AVMetadataCommonKeyTitle]) {
> +             setLabel((NSString*)item.value);
> +            break;
> +        }
> +    }

It would be good to return the label in the user's preferred language if possible. See InbandTextTrackPrivateLegacyAVFObjC::label.

> Source/WebCore/platform/graphics/avfoundation/objc/AudioTrackPrivateAVFObjC.mm:95
> +    setLanguage([assetTrack extendedLanguageTag]);

This will miss a language stored as a QuickTime 5-bit packed code. See MediaPlayerPrivateAVFoundationObjC::languageOfPrimaryAudioTrack.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:220
> +    Vector<RefPtr<AudioTrackPrivateAVFObjC> > m_audioTracks;

We don't need a space in "> >" any more.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1060
> +        typedef Vector<RefPtr<AudioTrackPrivateAVFObjC> > AudioTrackVector;

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1073
> +                player()->removeAudioTrack(i->get());

It would probably be safer to so this after the m_audioTracks.swap, so the new track configuration is already set up.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1081
> +            player()->addAudioTrack(newTrack.get());

Ditto.
Comment 5 Jer Noble 2013-09-30 08:17:05 PDT
Committed r156652: <http://trac.webkit.org/changeset/156652>