Azure / Communication

Azure Communication Services - SDKs and Release Notes
MIT License
317 stars 98 forks source link

Call recording seems to ignore camera orientation/aspect #363

Open tompi opened 3 years ago

tompi commented 3 years ago

Call recording is really cool!

Minor issue: the screenshot attached is from a recording of a group call, with just one participant(iphone). This participant had his camera in portrait mode, but recording seems to cut top and bottom and force a landscape frame from the "middle" of the original feed. I hope there can be a way to specify that you want "aspectfit" for participants feeds!

Also: the feed seemed more grainy and with less quality than what we expected, is there a loss of resolution when saving the video?

ACS_Recording

RinaRish commented 3 years ago

Hi @tompi! Thanks for reporting the issue. AB#2568177

chrwhit commented 3 years ago

@tompi I'm looking into this issue, and I just wanted to check on one thing. Are you using the JavaScript SDK from a browser on iOS?

tompi commented 2 years ago

@chrwhit Hi, we are using the native iOS SDK

RinaRish commented 2 years ago

@tompi was the camera translating the accurate layout before recording has started? just to confirm that it is the recording that caused the bug

tompi commented 2 years ago

@RinaRish Yes, looked fine in the "preview" on the local device, and on the "other end"(which in this case was a web frontend). It seems like the recording is using a fixed aspect ratio(16/9) for "cutting" the video.

This is what the teams client also usually does. But as we are making our own "client" we dont have to, this is what we do in the javascript client:

     const view = await this.rendererRemote.createView({
        isMirrored: true,
        scalingMode: "Fit" /* Fit | Stretch | Crop*/
      });

The keyword here is we use "Fit", while Im assuming the serverside recording code uses "Crop".

Here is an example of the same rendering for native SDK(iOS):

   var renderer = new ACSVideoStreamRenderer(acsRemoteVideoStream, out var rendererError);
                _nsErrorHelper.ThrowIfError(rendererError);
                var renderingOptions = new ACSCreateViewOptions(_settings.CropVideo ? ACSScalingMode.Crop : ACSScalingMode.Fit);
                var nativeView = renderer.CreateViewWithOptions(renderingOptions, out var createViewError);

I understand this is a non-trivial issue if you are mixing all videos into 1. For most usecases it will probably be okay to crop the image, but in out case cropping a portrait stream cuts out the most important thing for us(the bottom part...). So I suggest you just expose this as an option in the recording API, like you do in the rendering API.

chrwhit commented 2 years ago

@tompi I looked into the issue, portrait video is being ingested; however, our recording service currently only supports landscape video, and crops the portrait video to fit this requirement. This is a current limitation. If this is something important to your scenario, we can discuss adding this as a future feature.

tompi commented 2 years ago

Thank you @chrwhit , thats what we suspected. I attached a screenshot of our web frontend, which has the ACS video embedded. Our main focus is the bottom of the portrait screen, where the patient(in this case CPR manikin...) is located and is receiving CPR...

Please let us know if we need to push for this feature somewhere else than this board!

Screenshot 2021-09-29 at 12 23 52

RinaRish commented 2 years ago

Hi @tompi! So we are going to close this since your ask is a feature and for now, there is no ETA. Can you please reach to the customer engagement team through the TAP program so your feedback is being captured? Thanks

tompi commented 2 years ago

@RinaRish Im gonna have to disagree with this being a feature ask and not a bug ;) The recording of the meeting shows an arbitrary crop of the video... It's kind of the point of a recording to reflect what you saw during the call, right?

This bug will affect all recordings of meetings that has at least one participant with a portrait oriented video stream... I understand that the most common usecase is a landscape webcam stream, but I also assume it will be more and more common to use your phones for teams/acs, so maybe worth fixing this now instead of later?

RinaRish commented 2 years ago

AB#2637074

tompi commented 2 years ago

Hey @RinaRish ! Do you know the status of the internal issue? (I dont have access...) (My PM is eager to avoid the cropping...)

curelom commented 2 years ago

I've got to agree with tompi. This shouldn't be considered a feature, but a bug. When recording with a phone in portrait mode a very large amount of the video is cropped making it unusable. At best we would have to manually overlay the video so the user knows what section of the video is actually being recorded.

curelom commented 2 years ago

AB#2637074

Can you grant access to this? Says I don't have access