bug: CameraSource::bufferCaptured messes up aspect ratio

[jgh-]

bug: CameraSource::bufferCaptured messes up aspect ratio

[jgh-]

Hm, I couldn't really tell from your video what exactly the problem is. Here's what's happening in that code though:

• On the first frame it receives, it will check the aspect ratio of the frame (m_size).
• It will attempt to put an aspect-fit image of the video into the bounding box provided (m_target_size, this is the size specified in the constructor)
• The compositor uses a normalized device coordinate system, meaning 0,0 is at the center, -1,-1 is lower left, and 1,1 is upper right. That is where the .x / target_size.vw * 2 - 1 comes from. (divide by the width of the video to get the normalized coordinate, and then multiply by 2 and subtract 1 to get the -1..1 value)

It's possible that I messed something up when switching to matrices instead of the session properties as before, so I'll take a look at it when i get home.

[jeffreywescott]

There are a few problems, and the behavior is inconsistent depending on whether the stream starts in landscape or portrait:

1. "Aspect fit" isn't working. It's distorting the shape of the video in some cases.
2. The bounding box is being shrunk in the course of switching back-and-forth between orientations.

[jgh-]

Can you post a capture of the video coming off the server? I'm curious if maybe the in-app display might be confounding this a bit

[jeffreywescott]

It would take me a bit to do so, but that's not the issue. I know because I was simultaneously watching the stream in a browser.

On Mon, Jun 9, 2014 at 6:36 PM, James Hurley [email protected]
wrote:

Can you post a capture of the video coming off the server? I'm curious if maybe the in-app display might be confounding this a bit

Reply to this email directly or view it on GitHub:
https://github.com/jamesghurley/VideoCore/issues/20#issuecomment-45564903

[jgh-]

OK. You know what, I'm going to change the way this works a bit so that we can just use pixels as a unit of measurement in the sources and transforms instead of using the normalized device coordinates off the bat. I'll add an orthographic project matrix in the compositor and it should simplify things to prevent bugs like this in the future.

[jeffreywescott]

Okay. 

FWIW, I think it's the calculations on m_size that aren't right. Perhaps dividing by the quotient of the two aspect ratios isn't what was intended?

On Mon, Jun 9, 2014 at 7:09 PM, James Hurley [email protected]
wrote:

OK. You know what, I'm going to change the way this works a bit so that we can just use pixels as a unit of measurement in the sources and transforms instead of using the normalized device coordinates off the bat. I'll add an orthographic project matrix in the compositor and it should simplify things to prevent bugs like this in the future.

Reply to this email directly or view it on GitHub:
https://github.com/jamesghurley/VideoCore/issues/20#issuecomment-45566581

[jgh-]

Ah, I think I found the bug. The height should be multiplied by the quotient instead of divided.

based on some rough scenarios :

480x320 = 1.5       (target)
640x480 = 1.33333       (input from camera)

expected size

426x320 = 1.33125

diff = 1.5 / 1.33333, updated width since target > input
w =  480 / 1.125 = 426
————————————


480x320 = 1.5       (target)
1280x720 = 1.77777      (input from camera)

expected size

480x270 = 1.777777

diff = 1.5 / 1.777777, update height since target < input
h = 320 / 0.84375 = 379 <— incorrect (current code)
h = 320 * 0.84375 = 270 <— correct

[jgh-]

OK that should be it. Sleeping on it made me realize the problem with the shrinking bounding box.

[jeffreywescott]

Thanks, James! I'll give it a try today.

Any chance you could bump the version and upload the new .podspec (if you haven't already)?

[jgh-]

Will do.

On Tue, Jun 10, 2014 at 9:20 AM, Jeffrey Wescott [email protected]
wrote:

Thanks, James! I'll give it a try today.

Any chance you could bump the version and upload the new .podspec (if you
haven't already)?

Reply to this email directly or view it on GitHub
https://github.com/jamesghurley/VideoCore/issues/20#issuecomment-45636387
.

[notedit]

@jeffreywescott so i recommend set the pod to dev mod when you are in dev:

pod 'VideoCore', :path => 'VideoCore/VideoCore.podspec'

[jeffreywescott]

Just tried it again. Not much better, actually. Picture still skewed.

Input: landscape, 568x320
Output: 757x320

Input: portrait, 320x568
Output: 180x568

This time, it actually skews things even before I rotate. The numbers above were pre-rotation.

[jeffreywescott]

@notedit -- I already do that. Thanks.

[jgh-]

Yeah I'm seeing that now. I have the SampleBroadcaster up and running in front of me so I should have a fix in a few minutes.

[jgh-]

OK looks like the quotient does need to be there (I wasn't crazy when I put that in there!) to compensate for aspect ratios less than 1.

I have rolled back the code to use the quotient of the two aspect ratios again, and it will reset the m_size to be m_target_size each time so it should not accumulate errors.

James

[jeffreywescott]

Now it's more or less back to the original bug. <sigh>

landscape in: 568x320
landscape out: 427x320 (okay given the quality setting)
rotated to portrait: 240x320 (too small)

portrait in: 320x568
portrait out: 320x568 (okay)
rotated to landscape: 320x1792 (way off)

[jeffreywescott]

FWIW, here's some code I've used in the past for "aspect fit". I apologize for the ActionScript:

    private function getVideoDimensions():Object {
      log("Stage dimensions:", stage.stageWidth, "x", stage.stageHeight);
      var width:int, height:int;
      var stageAR:Number = stage.stageWidth / stage.stageHeight;
      var streamAR:Number = this.options.streamWidth / this.options.streamHeight;
      if (streamAR >= stageAR) { // too wide
        width = stage.stageWidth;
        height = Math.round(width / streamAR);
      } else if (streamAR < stageAR) { // too tall
        height = stage.stageHeight;
        width = Math.round(height * streamAR);
      }

      return {
        width: width
      , height: height
      };
    }

[jgh-]

hum... this is working pretty much perfectly for me right now

Target size: (1280.000000, 720.000000) Calculated size: (405.000000, 720.000000) Input size: (720.000000, 1280.000000) Aspect: (1.777778) Input aspect: (0.562500)
2014-06-10 11:00:56.797 SampleBroadcaster[515:1307] Connection status: 1
2014-06-10 11:00:56.802 SampleBroadcaster[515:1307] Connected
Target size: (1280.000000, 720.000000) Calculated size: (1280.000000, 720.000000) Input size: (1280.000000, 720.000000) Aspect: (1.777778) Input aspect: (1.777778)
Target size: (1280.000000, 720.000000) Calculated size: (405.000000, 720.000000) Input size: (720.000000, 1280.000000) Aspect: (1.777778) Input aspect: (0.562500)

I'm going to download your code and see if I can reproduce.

[jgh-]

Well, I will download your code as soon as github stops giving me 500 errors.

[jeffreywescott]

It's the rotation that makes things screwy, I think. Standard "aspect fit" algorithms fall apart because height and width have been swapped.

Perhaps reorientCamera needs to swap target_w and target_h?

[jgh-]

I don't think so because the outgoing video isn't changing its dimensions, just the incoming image from the camera so it should just use the aspect ratio of the incoming image and the aspect ratio of the bounding box (target).

However in my testing I'm seeing it correctly calculate the new image size for the incoming image to fit within the bounding box (Target size is 1280x720, when in Landscape it's 1280x720 so no change and when it's portrait it's 720x1280, which results in a 405x720 image and that is the same aspect ratio as 720x1280)

I'm finally able to download your code so I'm setting that up so at least we can be looking at the same code.

[jgh-]

Oh very strange! This code doesn't behave like this in the Sample App for me, but it definitely is screwed up in your app. OK now I've got something to work with.

[jeffreywescott]

Perhaps it's something in my code? I just tried 1280x720 and also get an output size of 405x720 with my code. However, it ends up rotating it 90 degrees and shrinking itself in the NSImageView. Super weird.

FWIW, my NSImageView is just pinned to the outer view so that it's always full-screen. So when it rotates, its dimension changes. Perhaps that's the issue?

I'm just trying to make the thing behave more or less like the Video section of the iOS Camera app.

[jgh-]

OK I think I see what's happening here. There are a few issues:

1. Starting the app in Portrait messes it up since you get the video dimensions from the dimensions of the phone. A better way to do it would be to pick the resolutions you wish to use and stick with those. BUT this is revealing a serious bug in that code: if the video is portrait, it doesn't handle it very well. The aspect ratios work perfectly if the video is landscape.
2. Your NSImageView is showing the resulting image coming off of the compositor. So when you rotate the camera you are ending up with an aspect-fit image and then aspect fitting it, so you end up with a really little image in the middle heh. One thing I'm going to do is add a hook for AVCaptureVideoPreviewLayer in the CameraSource so that for applications where the user doesn't care about the composited image (such as yours) as much, they'll have a better experience
3. I am using the AVCaptureSessionPresetLow, AVCaptureSessionPresetMedium, and AVCaptureSessionPresetHigh presets for setting up the session. These result in different aspect ratios depending on the device. AVCaptureSessionPresetLow tends to be more like 4:3 and AVCaptureSessionPresetHigh tends to be more like 16:9

I'll work on putting that preview layer in.

[jeffreywescott]

Thanks, James. 

FWIW, as cine.io is really meant to be a streaming service, I have no way of predetermining the "supported" video resolutions.

It seems to me that if the target resolution is landscape, and the user is in portrait, it should just letterbox, no? Likewise for the reverse case.

On Tue, Jun 10, 2014 at 11:38 AM, James Hurley [email protected]
wrote:

OK I think I see what's happening here. There are a few issues:

1. Starting the app in Portrait messes it up since you get the video dimensions from the dimensions of the phone. A better way to do it would be to pick the resolutions you wish to use and stick with those.
   BUT this is revealing a serious bug in that code: if the video is portrait, it doesn't handle it very well. The aspect ratios work perfectly if the video is landscape.
2. Your NSImageView is showing the resulting image coming off of the compositor. So when you rotate the camera you are ending up with an aspect-fit image and then aspect fitting it, so you end up with a really little image in the middle heh. One thing I'm going to do is add a hook for AVCaptureVideoPreviewLayer in the CameraSource so that for applications where the user doesn't care about the composited image (such as yours) as much, they'll have a better experience

I'll work on putting that preview layer in.

Reply to this email directly or view it on GitHub:
https://github.com/jamesghurley/VideoCore/issues/20#issuecomment-45653917

[jgh-]

Right. That's how it works right now. For example, this is 1280x720 with the device rotated to portrait and then landscape.

It should be reversed if the output video is portrait, however the bug in the camera aspect ratio code is preventing that right now.

As for supported resolutions, do you guys have any sort of negotiation for supported resolutions on the service? Or are you just sort of taking whatever people throw at you?

[jeffreywescott]

We can take anything they toss our way. The objective is streaming from any device to any other device.

On Tue, Jun 10, 2014 at 12:37 PM, James Hurley [email protected]
wrote:

Right. That's how it works right now. For example, this is 1280x720 with the device rotated to portrait and then landscape.


It should be reversed if the output video is portrait, however the bug in the camera aspect ratio code is preventing that right now.

As for supported resolutions, do you guys have any sort of negotiation for supported resolutions on the service? Or are you just sort of taking whatever people throw at you?

Reply to this email directly or view it on GitHub:
https://github.com/jamesghurley/VideoCore/issues/20#issuecomment-45661201

[jgh-]

OK. I will fix this bug (since it needs to be fixed anyway, this is not a good one to leave). A feature I will be adding in the near future is adaptive streaming which may help you select resolution and bit rate combinations.

[jeffreywescott]

Cool!

[notedit]

in fact, i have came to this before #8 ,

i just change

https://github.com/jamesghurley/VideoCore/blob/master/sample/SampleBroadcaster/SampleBroadcaster/SampleGraph.cpp#L157

 m_cameraSource = std::make_shared<videocore::iOS::CameraSource>(frame_w/2,frame_h/2, frame_w,frame_h, frame_w, frame_h, float(frame_w) / float(frame_h));

to

 m_cameraSource = std::make_shared<videocore::iOS::CameraSource>(frame_w/2,frame_h/2, frame_w,frame_h, frame_w, frame_h, float(frame_h) / float(frame_w));

and it works, you have changed a lot in here, i don't know if it still works now.

[jgh-]

There's most definitely a bug in the CameraSource aspect ratio code, what you did ensures it works within the scope of the code that's working, but the bug is most visible when outputting a portrait video.

[jgh-]

@jeffreywescott I have revamped the aspect fit algorithm and it should work for both orientations now. It's working ok for me right now in the sample app (tested both portrait and landscape output movie)

[jeffreywescott]

Unfortunately, not for me. I have the output set to 720x1280 (portrait). It works for portrait, but when I turn the phone, landscape is out of whack:

[jgh-]

I think this is due to the fact that you're pulling the image off of the compositor (which will be 720x1280, the same as the video output). It looks like the aspect ratio is actually correct, just the window is tiny because it's being aspect fit into a portrait frame and then a landscape frame.

I just merged @notedit's contribution of a preview layer, I recommend giving that a try!

[jeffreywescott]

No ... look again. The image that is coming off is landscape (which is not right). It should be letter-boxed and portrait if the target is 720x1280, no?

[jgh-]

No, the camera's orientation is physically Landscape Left IIRC. So you're not going to be able to get a portrait image in landscape mode.

In the camera source there's a "reorientCamera" method that is called when the device's orientation changes, but all this really does is tells AVFoundation which way to orient the emitted image. If I didn't call that, all images would be landscape left.

So what it looks like to me is happening is you have a 720x1280 outputted video, and it rotates so the video input is 1280x720, so that gets aspect fit into the outputted video. And then your preview view is aspect fitting again so that little letterbox video gets letterboxed again because now you're fitting a 720x1280 image into a 1152x640 view.

What we could do is make an aspect fill mode that would at least leave the image larger, but then you're clipping a lot of the incoming video.

[jgh-]

So doing some quick back of the napkin math a 1280x720 image in a 720x1280 framebuffer in a 1152x640 view should result in

(fb_w)720 / (img_w) 1280 = 0.5625 -> (1280*0.5625, 720*0.5625) -> 720x405 image 
(v_h)640 / (fb_h)1280 = 0.5 -> (720*0.5, 1280*0.5) -> 360x640 image

So that 360x640 we end up with in the view is the entire image coming off the compositor. The fitted image should be (720*0.5, 405*0.5) = (360, 202.5) which matches what is in your screenshot.

[jgh-]

@jeffreywescott I've added CameraSource::setAspectMode({kAspectFit, kAspectFill}) so you can do aspect fill and end up with a filled image instead of letterbox.

[jeffreywescott]

That's an awesome change, @jamesghurley! Thanks for that!

[jeffreywescott]

Any chance of a 0.1.5 version bump? :-D

[jgh-]

Will do when I get a few minutes to put it together

[jgh-]

0.1.5 pushed with Aspect Fill. Closing this now.