You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by muratsu <gi...@git.apache.org> on 2015/05/13 01:26:46 UTC

[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

GitHub user muratsu opened a pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94

    CB-8879 fix stripe issue with correct aspect ratio

    This fixes the stripe issue we had on windows phone 8+. 
    The fix was to set up resolutions with a common aspect ratio. Default aspect ratio is 1.78 (hd). 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/MSOpenTech/cordova-plugin-camera CB-8879

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-plugin-camera/pull/94.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #94
    
----
commit 4710faa91d86e46df1d66c24080b55cd8d86d459
Author: Murat Sutunc <su...@gmail.com>
Date:   2015-05-12T00:09:43Z

    CB-8879 fix stripe issue with correct aspect ratio

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by muratsu <gi...@git.apache.org>.
Github user muratsu closed the pull request at:

    https://github.com/apache/cordova-plugin-camera/pull/94


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30210599
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -337,28 +336,48 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                     }
                 });
     
    -            capture.initializeAsync(captureSettings).done(function () {
    -                // msdn.microsoft.com/en-us/library/windows/apps/hh452807.aspx
    -                capturePreview.msZoom = true;
    -                capturePreview.src = URL.createObjectURL(capture);
    -                capturePreview.play();
    +            captureSettings.photoCaptureSource = Windows.Media.Capture.PhotoCaptureSource.photo;
     
    -                // Insert preview frame and controls into page
    -                document.body.appendChild(capturePreview);
    -                document.body.appendChild(captureCancelButton);
    +            return capture.initializeAsync(captureSettings);
    +        }).then(function () {
    +            // msdn.microsoft.com/en-us/library/windows/apps/hh452807.aspx
    +            capturePreview.msZoom = true;
    +            capturePreview.src = URL.createObjectURL(capture);
    +            capturePreview.play();
     
    -                // Bind events to controls
    -                window.addEventListener('deviceorientation', cameraPreviewOrientation, false);
    -                capturePreview.addEventListener('click', captureAction);
    -                captureCancelButton.addEventListener('click', function () {
    -                    destroyCameraPreview();
    -                    errorCallback('Cancelled');
    -                }, false);
    -            }, function (err) {
    +            // Insert preview frame and controls into page
    +            document.body.appendChild(capturePreview);
    +            document.body.appendChild(captureCancelButton);
    +
    +            // Bind events to controls
    +            window.addEventListener('deviceorientation', cameraPreviewOrientation, false);
    +            capturePreview.addEventListener('click', captureAction);
    +            captureCancelButton.addEventListener('click', function () {
                     destroyCameraPreview();
    -                errorCallback('Camera intitialization error ' + err);
    -            });
    -        }, errorCallback);
    +                errorCallback('Cancelled');
    +            }, false);
    +
    +            // Get available aspect ratios
    +            var aspectRatios = getAspectRatios(capture);
    +
    +            // Couldn't find a good ratio
    +            if (aspectRatios.length === 0) {
    +                destroyCameraPreview();
    +                errorCallback('There\'s not a good aspect ratio available');
    +                return;
    +            }
    +
    +            // Default aspect ratio 1.78 (16:9 hd video standard)
    +            if (aspectRatios.indexOf("1.8") > -1) {
    --- End diff --
    
    IMO it will be better to use constant here instead: `DEFAULT_ASPECT_RATIO = 1.8`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30210477
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -438,6 +457,61 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                 });
         };
     
    +    var getAspectRatios = function (capture) {
    +        var photoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.photo).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var videoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.videoRecord).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var videoPreviewAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.videoPreview).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var allAspectRatios = [].concat(photoAspectRatios, videoAspectRatios, videoPreviewAspectRatios);
    +
    +        var aspectObj = allAspectRatios.reduce(function (map, item) {
    +            if (!map[item]) {
    +                map[item] = 0;
    +            }
    +            map[item]++;
    +            return map;
    +        }, {});
    +
    +        return Object.keys(aspectObj).filter(function (k) {
    +            return aspectObj[k] === 3;
    --- End diff --
    
    You are restricting aspect ratios to those which only supported by __all__ capture modes (picture, video and videoPreview). Not sure if it is by intent.
    
    Probably it makes sense to respect __current__ capture mode and return aspect ratios for this capture mode. Potentially there could be a situation when device's camera support a different aspect ratios for video and image capture.
    
    For example video is captured only in 640*480, which is equal to 1.3 aspect ratio, and for image there could be a number of other aspect ratios). This logic will return only '1.3' or even an empty set.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30210493
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -438,6 +457,61 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                 });
         };
     
    +    var getAspectRatios = function (capture) {
    +        var photoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.photo).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    --- End diff --
    
    nit: it should be enough simply return result of comparison from filter function, like `return index === array.indexOf(element)`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by robpaveza <gi...@git.apache.org>.
Github user robpaveza commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30254781
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -337,28 +336,48 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                     }
                 });
     
    -            capture.initializeAsync(captureSettings).done(function () {
    -                // msdn.microsoft.com/en-us/library/windows/apps/hh452807.aspx
    -                capturePreview.msZoom = true;
    -                capturePreview.src = URL.createObjectURL(capture);
    -                capturePreview.play();
    +            captureSettings.photoCaptureSource = Windows.Media.Capture.PhotoCaptureSource.photo;
     
    -                // Insert preview frame and controls into page
    -                document.body.appendChild(capturePreview);
    -                document.body.appendChild(captureCancelButton);
    +            return capture.initializeAsync(captureSettings);
    +        }).then(function () {
    +            // msdn.microsoft.com/en-us/library/windows/apps/hh452807.aspx
    +            capturePreview.msZoom = true;
    +            capturePreview.src = URL.createObjectURL(capture);
    +            capturePreview.play();
     
    -                // Bind events to controls
    -                window.addEventListener('deviceorientation', cameraPreviewOrientation, false);
    -                capturePreview.addEventListener('click', captureAction);
    -                captureCancelButton.addEventListener('click', function () {
    -                    destroyCameraPreview();
    -                    errorCallback('Cancelled');
    -                }, false);
    -            }, function (err) {
    +            // Insert preview frame and controls into page
    +            document.body.appendChild(capturePreview);
    +            document.body.appendChild(captureCancelButton);
    +
    +            // Bind events to controls
    +            window.addEventListener('deviceorientation', cameraPreviewOrientation, false);
    +            capturePreview.addEventListener('click', captureAction);
    +            captureCancelButton.addEventListener('click', function () {
                     destroyCameraPreview();
    -                errorCallback('Camera intitialization error ' + err);
    -            });
    -        }, errorCallback);
    +                errorCallback('Cancelled');
    +            }, false);
    +
    +            // Get available aspect ratios
    +            var aspectRatios = getAspectRatios(capture);
    +
    +            // Couldn't find a good ratio
    +            if (aspectRatios.length === 0) {
    +                destroyCameraPreview();
    +                errorCallback('There\'s not a good aspect ratio available');
    +                return;
    +            }
    +
    +            // Default aspect ratio 1.78 (16:9 hd video standard)
    +            if (aspectRatios.indexOf("1.8") > -1) {
    --- End diff --
    
    jshint rule violation: should be single quotes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by muratsu <gi...@git.apache.org>.
Github user muratsu commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30262317
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -438,6 +457,61 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                 });
         };
     
    +    var getAspectRatios = function (capture) {
    +        var photoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.photo).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var videoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.videoRecord).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var videoPreviewAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.videoPreview).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var allAspectRatios = [].concat(photoAspectRatios, videoAspectRatios, videoPreviewAspectRatios);
    +
    +        var aspectObj = allAspectRatios.reduce(function (map, item) {
    +            if (!map[item]) {
    +                map[item] = 0;
    +            }
    +            map[item]++;
    +            return map;
    +        }, {});
    +
    +        return Object.keys(aspectObj).filter(function (k) {
    +            return aspectObj[k] === 3;
    --- End diff --
    
    this is by design. all of the capture modes should be set to the same ratio otherwise there are artifacts on the captured image. this is a platform limitation. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30210499
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -438,6 +457,61 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                 });
         };
     
    +    var getAspectRatios = function (capture) {
    +        var photoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.photo).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var videoAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.videoRecord).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var videoPreviewAspectRatios = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.videoPreview).map(function (element) {
    +            return (element.width / element.height).toFixed(1);
    +        }).filter(function (element, index, array) { if (index === array.indexOf(element)) return 1; return 0; });
    +
    +        var allAspectRatios = [].concat(photoAspectRatios, videoAspectRatios, videoPreviewAspectRatios);
    +
    +        var aspectObj = allAspectRatios.reduce(function (map, item) {
    +            if (!map[item]) {
    +                map[item] = 0;
    +            }
    +            map[item]++;
    +            return map;
    +        }, {});
    +
    +        return Object.keys(aspectObj).filter(function (k) {
    +            return aspectObj[k] === 3;
    +        });
    +    };
    +
    +    var setAspectRatio = function (capture, aspect) {
    +        // Max photo resolution with desired aspect ratio
    +        var photoResolution = capture.videoDeviceController.getAvailableMediaStreamProperties(Windows.Media.Capture.MediaStreamType.photo).filter(function (elem) {
    +            return ((elem.width / elem.height).toFixed(1) === aspect) ? 1 : 0;
    --- End diff --
    
    nit: `return (elem.width / elem.height).toFixed(1) === aspect)` is sufficient for filtering, ternary operator is excess.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-camera pull request: CB-8879 fix stripe issue with ...

Posted by muratsu <gi...@git.apache.org>.
Github user muratsu commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-camera/pull/94#discussion_r30262108
  
    --- Diff: src/windows/CameraProxy.js ---
    @@ -337,28 +336,48 @@ function takePictureFromCameraWP(successCallback, errorCallback, args) {
                     }
                 });
     
    -            capture.initializeAsync(captureSettings).done(function () {
    -                // msdn.microsoft.com/en-us/library/windows/apps/hh452807.aspx
    -                capturePreview.msZoom = true;
    -                capturePreview.src = URL.createObjectURL(capture);
    -                capturePreview.play();
    +            captureSettings.photoCaptureSource = Windows.Media.Capture.PhotoCaptureSource.photo;
     
    -                // Insert preview frame and controls into page
    -                document.body.appendChild(capturePreview);
    -                document.body.appendChild(captureCancelButton);
    +            return capture.initializeAsync(captureSettings);
    +        }).then(function () {
    +            // msdn.microsoft.com/en-us/library/windows/apps/hh452807.aspx
    +            capturePreview.msZoom = true;
    +            capturePreview.src = URL.createObjectURL(capture);
    +            capturePreview.play();
     
    -                // Bind events to controls
    -                window.addEventListener('deviceorientation', cameraPreviewOrientation, false);
    -                capturePreview.addEventListener('click', captureAction);
    -                captureCancelButton.addEventListener('click', function () {
    -                    destroyCameraPreview();
    -                    errorCallback('Cancelled');
    -                }, false);
    -            }, function (err) {
    +            // Insert preview frame and controls into page
    +            document.body.appendChild(capturePreview);
    +            document.body.appendChild(captureCancelButton);
    +
    +            // Bind events to controls
    +            window.addEventListener('deviceorientation', cameraPreviewOrientation, false);
    +            capturePreview.addEventListener('click', captureAction);
    +            captureCancelButton.addEventListener('click', function () {
                     destroyCameraPreview();
    -                errorCallback('Camera intitialization error ' + err);
    -            });
    -        }, errorCallback);
    +                errorCallback('Cancelled');
    +            }, false);
    +
    +            // Get available aspect ratios
    +            var aspectRatios = getAspectRatios(capture);
    +
    +            // Couldn't find a good ratio
    +            if (aspectRatios.length === 0) {
    +                destroyCameraPreview();
    +                errorCallback('There\'s not a good aspect ratio available');
    +                return;
    +            }
    +
    +            // Default aspect ratio 1.78 (16:9 hd video standard)
    +            if (aspectRatios.indexOf("1.8") > -1) {
    --- End diff --
    
    i need to add jshint later to this repo


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org