filters: automatically flip video if displaymatrix indicates so #16090
Conversation
|
Note: Orientation files tested with: https://github.com/ianare/exif-samples/tree/master/jpg/orientation but I wasn't able to test on dmabuf_wayland. If someone could test that when reviewing that would be great. |
|
Download the artifacts for this pull request: |
video/mp_image.h
Outdated
| @@ -57,6 +57,8 @@ struct mp_image_params { | |||
|
|
|||
| enum mp_csp_light light; | |||
| enum pl_chroma_location chroma_location; | |||
| // The image should be flipped vertically before rotating | |||
| int vflip; | |||
There was a problem hiding this comment.
Any reason to not just use a bool here?
There was a problem hiding this comment.
btw. what about hflip? Maybe we should make it more complete while at it?
There was a problem hiding this comment.
no reason not to use a bool, mostly just habit of int as a bool. as for why not hflip, we also have rotate and a vflip and a rotation together can be an hflip. Since this is specifically "flip vertically before rotating" it creates ambiguity whereas hflip before rotation would require us to add 180 degrees to p->rotate.
There was a problem hiding this comment.
Also implementing a vflip in a driver is much easier for the most part, as we can negate linesize for some things (e.g. vo_gpu) but mirroring horizontally is more complicated.
video/out/vo_dmabuf_wayland.c
Outdated
| @@ -709,7 +710,7 @@ static int reconfig(struct vo *vo, struct mp_image *img) | |||
| vo->target_params = &p->target_params; | |||
| mp_mutex_unlock(&vo->params_mutex); | |||
|
|
|||
| wl_surface_set_buffer_transform(vo->wl->video_surface, img->params.rotate / 90); | |||
| wl_surface_set_buffer_transform(vo->wl->video_surface, img->params.rotate / 90 + 4 * !!img->params.vflip); | |||
There was a problem hiding this comment.
This almost works. On the samples in https://github.com/ianare/exif-samples/tree/master/jpg/orientation, 2 and 4 are upside down. It unfortunately looks like the ordering of the wayland transform enums swapped these two cases so a simple +4 won't work.
There was a problem hiding this comment.
When you say they are up-side-down, are they mirrored vertically, or are they rotated 180 degrees?
Also, according to the spec linked (which is where I referenced the +4), this should work, I wonder if the spec is wrong?
landscape_2 results in a displaymatrix:
displaymatrix=
00000000: -65536 0 0
00000001: 0 65536 0
00000002: 0 0 1073741824
Which has rotate set to 180 and vflip also set to true, which surprises me that this doesn't work. This should set the enum to 6, which is what landscape_2 does.
There was a problem hiding this comment.
They're rotated 180 degrees. swapping enum 4 and 6 give the expected result
There was a problem hiding this comment.
Yeah that. I tried sway/wlroots and weston and both have the same behavior.
There was a problem hiding this comment.
Does it work now? Added a simple LUT.
Currently we check AV_FRAME_DATA_DISPLAYMATRIX for its rotation factor but not for its flip factor, which could happen with some input files. This commit adds and autovflip filter, which is applied before the autorotate filter to flip vertically before rotating. It also changes the displaymatrix reading code to output to mp_image->params.vflip so it can be autoinserted as necessary. Signed-off-by: Leo Izen <leo.izen@gmail.com>
Currently we check AV_FRAME_DATA_DISPLAYMATRIX for its rotation factor but not for its flip factor, which could happen with some input files. This commit adds and autovflip filter, which is applied before the autorotate filter to flip vertically before rotating. It also changes the displaymatrix reading code to output to mp_image->params.vflip so it can be autoinserted as necessary.