Bug 122043 - [Mac] Add AudioTrack support to MediaPlayerPrivateAVFObC.
Summary: [Mac] Add AudioTrack support to MediaPlayerPrivateAVFObC.
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: InRadar
Depends on:
Blocks: 122049
  Show dependency treegraph
 
Reported: 2013-09-27 17:13 PDT by Jer Noble
Modified: 2013-09-30 08:17 PDT (History)
5 users (show)

See Also:


Attachments
Work in progress (22.10 KB, patch)
2013-09-27 17:16 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (26.16 KB, patch)
2013-09-27 17:32 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 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>