Bug 122122

Summary: [Mac] Add support for VideoTrack to MediaPlayerPrivateAVFObjC
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, glenn
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 122171    
Bug Blocks:    
Attachments:
Description Flags
Patch eric.carlson: review+

Description Jer Noble 2013-09-30 13:35:18 PDT
[Mac] Add support for VideoTrack to MediaPlayerPrivateAVFObjC
Comment 1 Jer Noble 2013-09-30 13:56:49 PDT
Created attachment 213033 [details]
Patch
Comment 2 Eric Carlson 2013-10-01 07:36:30 PDT
Comment on attachment 213033 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:83
> +    else

Nit: the final "else" isn't needed.

> Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:98
> +    else

Ditto.

> Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:121
> +    else

Ditto.

> Source/WebCore/platform/graphics/avfoundation/VideoTrackPrivateAVF.h:35
> +class VideoTrackPrivateAVF : public VideoTrackPrivate {

FINAL

> Source/WebCore/platform/graphics/avfoundation/VideoTrackPrivateAVF.h:42
> +    virtual Kind kind() const { return m_kind; }
> +    virtual AtomicString id() const { return m_id; }
> +    virtual AtomicString label() const { return m_label; }
> +    virtual AtomicString language() const { return m_language; }

OVERRIDE

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1134
> +void MediaPlayerPrivateAVFoundationObjC::updateVideoTracks()

This looks *awfully* similar to updateAudioTracks. Is there any way to share code like MediaPlayerPrivateAVFoundation::processNewAndRemovedTextTracks does for processing in-band text tracks?

> Source/WebCore/platform/graphics/avfoundation/objc/VideoTrackPrivateAVFObjC.h:40
> +class VideoTrackPrivateAVFObjC : public VideoTrackPrivateAVF {

FINAL

> Source/WebCore/platform/graphics/avfoundation/objc/VideoTrackPrivateAVFObjC.h:48
> +    virtual void setSelected(bool);

OVERRIDE

> LayoutTests/media/track/video-track.html:37
> +            function canplaythrough()
> +            {
> +                consoleWrite("<br><i>** Check initial in-band track states<" + "/i>");
> +                testExpected("video.videoTracks.length", 1);
> +
> +                consoleWrite("<br><i>** Unload video file, check track count<" + "/i>");
> +                run("video.src = ''");
> +                testExpected("video.videoTracks.length", 0);
> +
> +                consoleWrite("");
> +                endTest();
> +            }

This should also test the "only one video track enabled" spec requirement.
Comment 3 Jer Noble 2013-10-01 11:22:57 PDT
Committed r156722: <http://trac.webkit.org/changeset/156722>
Comment 4 WebKit Commit Bot 2013-10-01 12:36:10 PDT
Re-opened since this is blocked by bug 122171
Comment 5 Jer Noble 2013-10-22 16:55:27 PDT
In the end, the rollout was not necessary. Moving back to Resolved/Fixed.