[FFmpeg-devel] [PATCH] avutil/display: fix inverted doc

[Andreas Rheinhardt]

Zhao Zhili:
> ---
>  libavutil/display.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavutil/display.h b/libavutil/display.h
> index 515adad795..d87bf68425 100644
> --- a/libavutil/display.h
> +++ b/libavutil/display.h
> @@ -88,7 +88,7 @@
>  double av_display_rotation_get(const int32_t matrix[9]);
>  
>  /**
> - * Initialize a transformation matrix describing a pure counterclockwise
> + * Initialize a transformation matrix describing a pure clockwise
>   * rotation by the specified angle (in degrees).
>   *
>   * @param matrix an allocated transformation matrix (will be fully overwritten
> 

a) This change brings the documentation in line with actual behaviour.
b) This bug has been introduced in
e4fe535d12f4f30df2dd672e30304af112a5a827: The mov demuxer did not abide
by the documentation of the format of the display matrix side data (it
had an incorrect transposition). Instead of just fixing the mov demuxer
to abide by this format (which coincides with the native mov format!),
said commit also negated the degrees in av_display_rotation_get/set. In
case of av_display_rotation_get() this fixed a bug, in case of
av_display_rotation_set() this introduced a bug. No one seems to have
thought to actually test whether av_display_rotation_get() actually
returns the degree.
c) There is a test for this (libavutil/tests/display.c). It does not
perform the test just mentioned; instead it has the current behaviour
hardcoded in its ref file.
d) The rotate_override stuff in ffmpeg.c compensates for the weirdness
of av_display_rotation_set() by negating the degree. get_rotation in
fftools/cmdutils.c also negates the degree, but it seems to do so
deliberately in order to return a clockwise degree (both callers expect
it that way).
e) av_display_matrix_flip() is equivalent to multiplying the matrix with
a diagonal matrix (with diagonal entries +-1) from the right; given that
applying the displaymatrix corresponds to multiplying the coordinate row
vector with the matrix on the right, this means that flipping is applied
on top of the display matrix (i.e. after the transformation specified by
the earlier display matrix has been applied). This is in line with my
expectations of what it does given its documentation.
f) Yet it is not in line with how it is used by the H.2645 SEI parsing
code: According to the H.2645 specs, the flips should be applied first.
Flipping (regardless of axis) first, then rotating by phi is equivalent
to rotating by -phi first and then flipping around the same axis. This
implies that the code in libavcodec/hevcdec.c, libavcodec/h264_slice.c
and libavcodec/h264_metadata_bsf.c (when creating side data) leads to
correct results if there is exactly one flip; without a flip or two
flips, the sign of the degree is wrong. This is the reason for the issue
in your other mail [1].
g) The code to create an SEI from side data in h264_metadata_bsf.c is
incorrect.
h) I don't know whether the exif rotation code is correct; same for
android_camera.c.

- Andreas

[1]: http://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/289488.html