Bug 122122 - [Mac] Add support for VideoTrack to MediaPlayerPrivateAVFObjC
Summary: [Mac] Add support for VideoTrack to MediaPlayerPrivateAVFObjC
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:
Depends on: 122171
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-30 13:35 PDT by Jer Noble
Modified: 2013-10-22 16:55 PDT (History)
3 users (show)

See Also:


Attachments
Patch (49.55 KB, patch)
2013-09-30 13:56 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-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.