1275217 - Remove QTKit dependency

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P3)

Description

[Nils Ohlmeier [:drno]]

After bug 1180725 added support for using AVFoundation instead of QTKit and we no longer need to support Mac OSX 10.6 we should remove the QTKit related code and dependencies.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: MozReview Request: Bug 1275217: remove QuickTime and QTKit related code and dependecies. r=jib

Review commit: https://reviewboard.mozilla.org/r/54796/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54796/

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8755767 [details]
MozReview Request: Bug 1275217: remove QuickTime and QTKit related code and dependecies. r=jib

https://reviewboard.mozilla.org/r/54796/#review51558

Looks like you have some build errors on try...

Otherwise looks good, except we should maybe update video_capture_mac.mm as well to not mention qtkit (maybe remove 10.4 QuickTime as well)? Or maybe it serves as a historic record, I don't know.

[1] http://mxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/mac/video_capture_mac.mm#30

Comment 3

[Nils Ohlmeier [:drno]]

Comment on attachment 8755767 [details]
MozReview Request: Bug 1275217: remove QuickTime and QTKit related code and dependecies. r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54796/diff/1-2/

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8755767 [details]
MozReview Request: Bug 1275217: remove QuickTime and QTKit related code and dependecies. r=jib

https://reviewboard.mozilla.org/r/54796/#review51656

Comment 5

[Nils Ohlmeier [:drno]]

Comment on attachment 8755767 [details]
MozReview Request: Bug 1275217: remove QuickTime and QTKit related code and dependecies. r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54796/diff/2-3/

Comment 6

[Nils Ohlmeier [:drno]]

@:jib: I took your suggest from comment #2 to heart and massively cleaned up the code in video_capture_mac.mm as well (including replacing the deprecated Gestalt interface). If you could please have a look one more time and let me know if this is still r+ from you :-)

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

https://reviewboard.mozilla.org/r/54796/#review51916

(didn't notice review was unpublished. sorry for the delay)

Loving net negative patches. There's a bug in your version check, but r=me once that's fixed.

::: media/webrtc/trunk/webrtc/modules/video_capture/mac/video_capture_mac.mm:30
(Diff revision 3)
> -// static
> +static int intAtStringIndex(NSArray *array, int index)
> -bool CheckOSVersion()
>  {
> -    // Check OSX version
> +    return [(NSString *)[array objectAtIndex:index] integerValue];

Isn't a c-wrapper around the same call in Objective-C a bit redundant?

::: media/webrtc/trunk/webrtc/modules/video_capture/mac/video_capture_mac.mm:57
(Diff revision 3)
> -
> +    if (minor < 8) {
> -    if (version < 0x07000000) // QT v. 7.x or newer (QT 5.0.2 0x05020000)
> -    {
> -        WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceVideoCapture, 0,
> -                     "QuickTime version too old: 0x%x", version);
> -        return false;
> +      return false;
>      }

Wont this fail on e.g. version 11.7?

Comment 8

[Nils Ohlmeier [:drno]]

Comment on attachment 8755767 [details]
MozReview Request: Bug 1275217: remove QuickTime and QTKit related code and dependecies. r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54796/diff/3-4/

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fcedd1b3ef25

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fcedd1b3ef25