Is this a regression?

I used R64-10113.0.0 to test it and I can reproduce it.
(1) Open built in Camera app or WeVideo Android app.
(2) Switch to world facing camera
(3) Switch to tablet mode (flip the screen around).
(4) change to video recording mode
(5) record a video and replay

It looks like we need to rotate the stream in Android framework.

Sheng-hao is OOO this week. Henry. Please help check if this is a regression.

also reproduce on R62-9901.77.0 and R63-10032.37.0

Thanks. Henry. So this is not a regression. Assigning back to Sheng-hao.

I tested on reef 10113.0.0. Here's the findings. Everything is tested in tablet mode:

1. Orientations of preview/still capture work correctly for all rotations for both user and world cameras.

dsemaya@, if you see otherwise, it's probably because the firmware version of your reef needs updates.

2. Recorded videos in 90/180/270 are shown rotated in GooglePhoto player. They are shown correctly in Chrome.

I looked into the metadata of the mp4 files (attached). The "rotation" metadata are set correctly. I think the video player in GooglePhoto does not honor the rotation metadata. I guess desktop player like VLC 2.2.1 and above will play the videos correctly, as suggested in https://addpipe.com/blog/mp4-rotation-metadata-in-mobile-video-files/. I will confirm later since 2.2.1 is not available on current goobuntu.

What is the default video player on normal android devices? Chrome or GooglePhotos?

Deleted: video_0.mp4 2.3 MB

	video_0.mp4 2.3 MB View Download

Deleted: video_90.mp4 3.1 MB

	video_90.mp4 3.1 MB View Download

Deleted: video_180.mp4 2.7 MB

	video_180.mp4 2.7 MB View Download

Deleted: video_270.mp4 1.7 MB

	video_270.mp4 1.7 MB View Download

Default video player on normal android devices is GooglePhotos. You can try the videos on a pixel phone.

FWIW, I tried playing the videos on my Pixel phone and all four videos were rotated correctly.

Ricky and I verified that all 4 videos attached in #12 are displayed correctly on both GooglePhotos and Chrome in a normal android phone.

lpique@, do you know why the videos are rotated only on ARC++ GooglePhotos player while they are displayed correctly on ARC++ Chrome, normal android GooglePhotos player and Chrome?

The attached file is the "dumpsys SurfaceFlinger" result when playing video_90.mp4 in ARC++ in GooglePhotos.

Deleted: dump 23.5 KB

dump 23.5 KB View Download

Offhand I don't know why they would only be rotated for ARC++ GooglePhotos player. It almost sounds like an issue with the app?

From looking at the dump in #15, all the buffers that SurfaceFlinger sees have a mCurrentTransform of zero. All the buffer queues also have a transform-hint of zero, indicating that SF is being told by Chrome that the display has a rotation of zero. Both of those suggest that the Chrome compositor should not be doing anything to the output when compositing.

Devices such as the Reef I believe have accelerometers in both the lid and the base. With the base folded back, the accelerometer there would indicate the device is upside-down. Perhaps GooglePhotos is responding to that sensor reading?

It might be possible to not fold the base all the way back and still be in tablet mode, and have the app not flip the video.

We also have a tablet mode autotest that uses "ectool motionsense spoof" (Test code is here: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/client/site_tests/platform_TabletMode/platform_TabletMode.py) to spoof sensor values.

Another way of doing it might be to spoof the base accelerometer values with that tool, and observe how the app flips the videos in response. However I had limited success running those spoof commands on various Chromebooks. I remember only finding that it actually worked on one particular model, but I can't remember for sure which one.

It's not about the device rotation in tablet mode, but about the recorded video being rotated when played. We did the test on a Chromebook in laptop mode.

For the four videos in #12, video_90 and video_270 have dimension 1080x1920, and on GooglePhotos/Chrome on Pixel phone or Chrome on ARC++ the two videos are played correctly as 1080x1920. On GooglePhotos on ARC++, the video seems to be stretched from the original 1080x1920 format to a 1920x1080 buffer, and then displayed in portrait orientation, so the displayed video is rotated and stretched.

Looking at the SurfaceFlinger dumpsys, the GooglePhotos app has a 720x1280 layer that are showing 1280x720 buffers:

+ Layer 0xedae9800 (SurfaceView - com.google.android.apps.photos/com.google.android.apps.photos.home.HomeActivity)
  Region transparentRegion (this=0xedae9a5c, count=1)
    [  0,   0,   0,   0]
  Region visibleRegion (this=0xedae9808, count=1)
    [467,   0, 899, 768]
  Region surfaceDamageRegion (this=0xedae9844, count=1)
    [  0,   0,  -1,  -1]
      layerStack=   0, z=    21010, pos=(467,0), size=( 720,1280), crop=(   0,   0, 720,1280), finalCrop=(   0,   0,1366, 768), isOpaque=1, invalidate=0, alpha=1.000, flags=0x00000002, tr=[0.60, 0.00][0.00, 0.60]
      client=0xedc0d1c0
      format= 4, activeBuffer=[1280x 720:1280, 23], queued-frames=1, mRefreshPending=0
            mTexName=178 mCurrentTexture=8
            mCurrentCrop=[0,0,1280,720] mCurrentTransform=0
            mAbandoned=0
            -BufferQueue mMaxAcquiredBufferCount=2, mMaxDequeuedBufferCount=18, mDequeueBufferCannotBlock=0 mAsyncMode=0, default-size=[720x1280], default-format=4, transform-hint=00, FIFO(1)={09:0xedad6d40 crop=[0,0,1280,720], xform=0x00, time=0x4287f8586200, scale=SCALE_TO_WINDOW
}
             [00:0xef18a7c0] state=DEQUEUED, 0xedc0ae00 [1280x 720:1280, 23]
             [01:0xef18bd00] state=DEQUEUED, 0xedc0af80 [1280x 720:1280, 23]
             [02:0xef18b4e0] state=DEQUEUED, 0xedc0ac80 [1280x 720:1280, 23]
             [03:0xef18a360] state=DEQUEUED, 0xedc0a180 [1280x 720:1280, 23]
             [04:0xedad6ac0] state=DEQUEUED, 0xed612200 [1280x 720:1280, 23]
             [05:0xef18b1c0] state=DEQUEUED, 0xedc0a380 [1280x 720:1280, 23]
             [07:0xef18b620] state=DEQUEUED, 0xedc0ad00 [1280x 720:1280, 23]
            >[08:0xef18a540] state=ACQUIRED, 0xedc0a280 [1280x 720:1280, 23]
             [09:0xedad6d40] state=QUEUED  , 0xed612300 [1280x 720:1280, 23]
             [10:0xef18ac20] state=DEQUEUED, 0xedc0a480 [1280x 720:1280, 23]
             [11:0xef18a680] state=DEQUEUED, 0xedc0f080 [1280x 720:1280, 23]
             [12:0xef18aea0] state=DEQUEUED, 0xedc0ad80 [1280x 720:1280, 23]
             [13:0xef18aae0] state=DEQUEUED, 0xedc0a500 [1280x 720:1280, 23]
             [14:0xedad5ee0] state=DEQUEUED, 0xed612400 [1280x 720:1280, 23]
             [15:0xef189780] state=DEQUEUED, 0xedc0a880 [1280x 720:1280, 23]
             [16:0xef18a860] state=DEQUEUED, 0xedc0a900 [1280x 720:1280, 23]
             [17:0xef18afe0] state=DEQUEUED, 0xedc0ab80 [1280x 720:1280, 23]
             [18:0xef18acc0] state=DEQUEUED, 0xedc0a580 [1280x 720:1280, 23]
             [19:0xedad62a0] state=DEQUEUED, 0xed612480 [1280x 720:1280, 23]
             [06:0xef18be40] state=FREE    , 0xedc0f000 [1280x 720:1280, 23]

We were wondering if this is the expected behavior? Since GooglePhotos on Pixel phone plays the videos correctly, we were wondering if there's anything on the graphics and/or window management side that leads to the behavior we observed on GooglePhotos on ARC++.

+skuhne, in case he can think of something in WM/AM that might be affecting things.

Sure, but I was making a guess as to what could be causing it. Based on the original description in #1, I thought the video was rendered upside-down, and not being rotated 90 and squash-stretched.

I was pointing out the two transform values are zero to eliminate SurfaceFlinger/composition from affecting things.

The fact that a 1280x720 buffer is being used without rotation to fill a 720x1280 view rectangle pretty much has to be the reason for the distortion.

The question then is who is flipping the width and height.

I wrote a test app which did its own rendering, which looked at transform-hint and set mCurrentTransform appropriately itself, and also had to take the view width/height and possibly swap them to get the buffer width/height to allocate.

For most apps the EGL driver takes care of allocating the correct size buffer, and applying the transform-hint (if supported). For software rendered apps, I expect there is something in the framework to do something similar.

For media apps like GooglePhotos I would expect buffers to be allocated by the codec for playback, and by the camera hardware driver when using that as a source.

In this case I'd expect the buffers to be coming from the codec, and then re-used as is for display.

It's possible that the intent was that a rotated video source should set mCurrentTransform to a nonzero value, and it was expected that the value would be passed to SurfaceFlinger (but isn't). The other option might be that the codec should be applying a rotation, but isn't.

I went and playing video_0 and video_90 from #12 on my Pixel and looked at "dumpsys SurfaceFlinger" there, using ToT SF. For a pixel held at portrait orientation, for video_0 I observed that the Pixel HWC was being a asked to compose a layer with a buffer with no transform, and for video_90 the HWC was asked to compose a layer with a buffer with a 90 degree transform.

It seems likely then that that something like this should be happening on ARC++, but isn't.

I do not really think this is it, but one possibility is that the application only allows certain orientations. So if the application would be upside down (and you would not notice) that could explain it.

I would somehow guess that the following is happening:

JPEG's have a transform flag which tells the recording orientation. Vdeos have something similar: https://addpipe.com/blog/mp4-rotation-metadata-in-mobile-video-files/
I would guess now that some apps might simply strip these - or that the video decoder needs to be properly set up with this orientation. 
So in other words it is conceivable that some apps are "really rotating" the video hence it will be played correctly if the app supports it or not and some apps simply record the video as it comes off the sensor + the rotation in which it got recorded. Now - a 180 degree rotation is odd and I could easily see applications not interpreting this correctly.

However this is all only speculation and I was not investigating any further.

Hi lpique,

> I went and playing video_0 and video_90 from #12 on my Pixel and looked at "dumpsys SurfaceFlinger" there, using ToT SF. For a pixel held at portrait orientation, for video_0 I observed that the Pixel HWC was being a asked to compose a layer with a buffer with no transform, and for video_90 the HWC was asked to compose a layer with a buffer with a 90 degree transform.

> It seems likely then that that something like this should be happening on ARC++, but isn't.

Could you point me to the code piece that I can log the transform information of "HWC being asked to compose a layer with a buffer"? I can find out what the values on ARC++ are and probably trace up to the cause.

The buffer transform is retrieved by SurfaceFlinger via getCurrentTransform(), which returns GLConsumer::mCurrentTransform. That value is set here in GLConsumer::updateAndReleaseLocked():

https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/GLConsumer.cpp#492

I believe just up the call stack you should find the buffer transform is set here by a call to BufferQueueProducer::queueBuffer():

https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/BufferQueueProducer.cpp#761

This might not apply for codec buffers, but for GL rendering to a surface, the EGL implementation makes this call to set a buffer transform via Surface::setBuffersTransform() (Or perhaps following setBuffersStickyTransform())

https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/Surface.cpp#1466






If you need it, the transform hint set by SurfaceFlinger is set via GLConsumer::setTransformHint(), and is passed to the more opaque IGraphicBufferConsumer interface to be handled by the producer.

https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/GLConsumer.cpp#1134

For GL rendering, the EGL implementation makes a call to query the transform hint, which is handled by Surface::query()

https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/Surface.cpp#836

|transform| are all 0 in https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/BufferQueueProducer.cpp#761

I found that it's caused by https://android.googlesource.com/platform/frameworks/native/+/master/libs/gui/BufferQueueProducer.cpp#761

Owen, why does ArcCodec always set rotation to 0? Shouldn't we read the rotation metadata from video header?

The second link in #24 should be https://cs.corp.google.com/arc-nyc-mr1/frameworks/av/media/libstagefright/ArcCodec.cpp?l=719

It looks like a bug in ArcCodec, will have a fix for it.

Fixed by
https://googleplex-android-review.git.corp.google.com/c/platform/frameworks/av/+/3285457

In which version will this land?  How complex was the fix?  Can we get this merged into 63?

Not reproducible: Tested on Basking, Electro with build 10230.0.0, m65.0.3299.0 (dev)