[webkit-reviews] review granted: [Bug 122043] [Mac] Add AudioTrack support to MediaPlayerPrivateAVFObC. : [Attachment 212859] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Sep 28 13:28:13 PDT 2013
Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 122043: [Mac] Add AudioTrack support to MediaPlayerPrivateAVFObC.
https://bugs.webkit.org/show_bug.cgi?id=122043
Attachment 212859: Patch
https://bugs.webkit.org/attachment.cgi?id=212859&action=review
------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
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/MediaPlayerPrivateAVFoundati
onObjC.h:220
> + Vector<RefPtr<AudioTrackPrivateAVFObjC> > m_audioTracks;
We don't need a space in "> >" any more.
>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:1060
> + typedef Vector<RefPtr<AudioTrackPrivateAVFObjC> > AudioTrackVector;
Ditto.
>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.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/MediaPlayerPrivateAVFoundati
onObjC.mm:1081
> + player()->addAudioTrack(newTrack.get());
Ditto.
More information about the webkit-reviews
mailing list