| Message ID | AS8P250MB0744E8BE1C2515319ABA75A08F0FA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | c797b6400d20633df931d813297db1bac7a94d72 |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avformat/matroskaenc: Support rotations | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Andreas Rheinhardt: > Matroska supports orthogonal transformations (both pure rotations > as well as reflections) via its 3D-projection elements, namely > ProjectionPoseYaw (for a horizontal reflection) as well as > ProjectionPoseRoll (for rotations). This commit adds support > for this. > > Support for this in the demuxer has been added in > 937bb6bbc1e8654633737e69e403e95a37113058 and > the sample used in the matroska-dovi-write-config8 FATE-test > includes a displaymatrix indicating a rotation which is now > properly written and read, thereby providing coverage for > the relevant code in the muxer as well as the demuxer. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Honestly, I am not really sure how to handle the floating-point > inaccuracies here (in atan2). > > libavformat/matroskaenc.c | 100 +++++++++++++++++---- > tests/ref/fate/matroska-dovi-write-config8 | 13 ++- > 2 files changed, 94 insertions(+), 19 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 41e13b273d..c1f40b26e6 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -1403,25 +1403,75 @@ static void mkv_write_video_color(EbmlWriter *writer, const AVStream *st, > } > > #define MAX_VIDEO_PROJECTION_ELEMS 6 > -static void mkv_write_video_projection(AVFormatContext *s, EbmlWriter *writer, > - const AVStream *st, uint8_t private[]) > +static void mkv_handle_rotation(void *logctx, const AVStream *st, > + double *yaw, double *roll) > +{ > + const int32_t *matrix = > + (const int32_t*)av_stream_get_side_data(st, AV_PKT_DATA_DISPLAYMATRIX, NULL); > + > + if (!matrix) > + return; > + > + /* Check whether this is an affine transformation */ > + if (matrix[2] || matrix[5]) > + goto ignore; > + > + /* This together with the checks below test whether > + * the upper-left 2x2 matrix is nonsingular. */ > + if (!matrix[0] && !matrix[1]) > + goto ignore; > + > + /* We ignore the translation part of the matrix (matrix[6] and matrix[7]) > + * as well as any scaling, i.e. we only look at the upper left 2x2 matrix. > + * We only accept matrices that are an exact multiple of an orthogonal one. > + * Apart from the multiple, every such matrix can be obtained by > + * potentially flipping in the x-direction (corresponding to yaw = 180) > + * followed by a rotation of (say) an angle phi in the counterclockwise > + * direction. The upper-left 2x2 matrix then looks like this: > + * | (+/-)cos(phi) (-/+)sin(phi) | > + * scale * | | > + * | sin(phi) cos(phi) | > + * The first set of signs in the first row apply in case of no flipping, > + * the second set applies in case of flipping. */ > + > + /* The casts to int64_t are needed because -INT32_MIN doesn't fit > + * in an int32_t. */ > + if (matrix[0] == matrix[4] && -(int64_t)matrix[1] == matrix[3]) { > + /* No flipping case */ > + *yaw = 0; > + } else if (-(int64_t)matrix[0] == matrix[4] && matrix[1] == matrix[3]) { > + /* Horizontal flip */ > + *yaw = 180; > + } else { > +ignore: > + av_log(logctx, AV_LOG_INFO, "Ignoring display matrix indicating " > + "non-orthogonal transformation.\n"); > + return; > + } > + *roll = 180 / M_PI * atan2(matrix[3], matrix[4]); > + > + /* We do not write a ProjectionType element indicating "rectangular", > + * because this is the default value. */ > +} > + > +static int mkv_handle_spherical(void *logctx, EbmlWriter *writer, > + const AVStream *st, uint8_t private[], > + double *yaw, double *pitch, double *roll) > { > const AVSphericalMapping *spherical = > (const AVSphericalMapping *)av_stream_get_side_data(st, AV_PKT_DATA_SPHERICAL, > NULL); > > if (!spherical) > - return; > + return 0; > > if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR && > spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE && > spherical->projection != AV_SPHERICAL_CUBEMAP) { > - av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); > - return; > + av_log(logctx, AV_LOG_WARNING, "Unknown projection type\n"); > + return 0; > } > > - ebml_writer_open_master(writer, MATROSKA_ID_VIDEOPROJECTION); > - > switch (spherical->projection) { > case AV_SPHERICAL_EQUIRECTANGULAR: > case AV_SPHERICAL_EQUIRECTANGULAR_TILE: > @@ -1455,17 +1505,33 @@ static void mkv_write_video_projection(AVFormatContext *s, EbmlWriter *writer, > av_assert0(0); > } > > - if (spherical->yaw) > - ebml_writer_add_float(writer, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, > - (double) spherical->yaw / (1 << 16)); > - if (spherical->pitch) > - ebml_writer_add_float(writer, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, > - (double) spherical->pitch / (1 << 16)); > - if (spherical->roll) > - ebml_writer_add_float(writer, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, > - (double) spherical->roll / (1 << 16)); > + *yaw = (double) spherical->yaw / (1 << 16); > + *pitch = (double) spherical->pitch / (1 << 16); > + *roll = (double) spherical->roll / (1 << 16); > > - ebml_writer_close_master(writer); > + return 1; /* Projection included */ > +} > + > +static void mkv_write_video_projection(void *logctx, EbmlWriter *wr, > + const AVStream *st, uint8_t private[]) > +{ > + double yaw = 0, pitch = 0, roll = 0; > + int ret; > + > + ebml_writer_open_master(wr, MATROSKA_ID_VIDEOPROJECTION); > + > + ret = mkv_handle_spherical(logctx, wr, st, private, &yaw, &pitch, &roll); > + if (!ret) > + mkv_handle_rotation(logctx, st, &yaw, &roll); > + > + if (yaw) > + ebml_writer_add_float(wr, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, yaw); > + if (pitch) > + ebml_writer_add_float(wr, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, pitch); > + if (roll) > + ebml_writer_add_float(wr, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, roll); > + > + ebml_writer_close_or_discard_master(wr); > } > > #define MAX_FIELD_ORDER_ELEMS 2 > diff --git a/tests/ref/fate/matroska-dovi-write-config8 b/tests/ref/fate/matroska-dovi-write-config8 > index bb22563eee..58eb454865 100644 > --- a/tests/ref/fate/matroska-dovi-write-config8 > +++ b/tests/ref/fate/matroska-dovi-write-config8 > @@ -1,5 +1,5 @@ > -09ff3c0a038eec0cdf4773929b24f41a *tests/data/fate/matroska-dovi-write-config8.matroska > -3600606 tests/data/fate/matroska-dovi-write-config8.matroska > +80d2b23a6f27ab28b02a907b37b9649c *tests/data/fate/matroska-dovi-write-config8.matroska > +3600620 tests/data/fate/matroska-dovi-write-config8.matroska > #extradata 0: 551, 0xa18acf66 > #extradata 1: 2, 0x00340022 > #tb 0: 1/1000 > @@ -46,6 +46,15 @@ > 1, 395, 395, 23, 439, 0x7d85e4c9 > [STREAM] > [SIDE_DATA] > +side_data_type=Display Matrix > +displaymatrix= > +00000000: 0 65536 0 > +00000001: -65536 0 0 > +00000002: 0 0 1073741824 > + > +rotation=-90 > +[/SIDE_DATA] > +[SIDE_DATA] > side_data_type=DOVI configuration record > dv_version_major=1 > dv_version_minor=0 Will apply patches 1-5 of this patchset tomorrow unless there are objections. Notice that I slightly modified the test added in 3/5 to now also set a creation_time metadata to test a previously untested codepath. - Andreas
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 41e13b273d..c1f40b26e6 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1403,25 +1403,75 @@ static void mkv_write_video_color(EbmlWriter *writer, const AVStream *st, } #define MAX_VIDEO_PROJECTION_ELEMS 6 -static void mkv_write_video_projection(AVFormatContext *s, EbmlWriter *writer, - const AVStream *st, uint8_t private[]) +static void mkv_handle_rotation(void *logctx, const AVStream *st, + double *yaw, double *roll) +{ + const int32_t *matrix = + (const int32_t*)av_stream_get_side_data(st, AV_PKT_DATA_DISPLAYMATRIX, NULL); + + if (!matrix) + return; + + /* Check whether this is an affine transformation */ + if (matrix[2] || matrix[5]) + goto ignore; + + /* This together with the checks below test whether + * the upper-left 2x2 matrix is nonsingular. */ + if (!matrix[0] && !matrix[1]) + goto ignore; + + /* We ignore the translation part of the matrix (matrix[6] and matrix[7]) + * as well as any scaling, i.e. we only look at the upper left 2x2 matrix. + * We only accept matrices that are an exact multiple of an orthogonal one. + * Apart from the multiple, every such matrix can be obtained by + * potentially flipping in the x-direction (corresponding to yaw = 180) + * followed by a rotation of (say) an angle phi in the counterclockwise + * direction. The upper-left 2x2 matrix then looks like this: + * | (+/-)cos(phi) (-/+)sin(phi) | + * scale * | | + * | sin(phi) cos(phi) | + * The first set of signs in the first row apply in case of no flipping, + * the second set applies in case of flipping. */ + + /* The casts to int64_t are needed because -INT32_MIN doesn't fit + * in an int32_t. */ + if (matrix[0] == matrix[4] && -(int64_t)matrix[1] == matrix[3]) { + /* No flipping case */ + *yaw = 0; + } else if (-(int64_t)matrix[0] == matrix[4] && matrix[1] == matrix[3]) { + /* Horizontal flip */ + *yaw = 180; + } else { +ignore: + av_log(logctx, AV_LOG_INFO, "Ignoring display matrix indicating " + "non-orthogonal transformation.\n"); + return; + } + *roll = 180 / M_PI * atan2(matrix[3], matrix[4]); + + /* We do not write a ProjectionType element indicating "rectangular", + * because this is the default value. */ +} + +static int mkv_handle_spherical(void *logctx, EbmlWriter *writer, + const AVStream *st, uint8_t private[], + double *yaw, double *pitch, double *roll) { const AVSphericalMapping *spherical = (const AVSphericalMapping *)av_stream_get_side_data(st, AV_PKT_DATA_SPHERICAL, NULL); if (!spherical) - return; + return 0; if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR && spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE && spherical->projection != AV_SPHERICAL_CUBEMAP) { - av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); - return; + av_log(logctx, AV_LOG_WARNING, "Unknown projection type\n"); + return 0; } - ebml_writer_open_master(writer, MATROSKA_ID_VIDEOPROJECTION); - switch (spherical->projection) { case AV_SPHERICAL_EQUIRECTANGULAR: case AV_SPHERICAL_EQUIRECTANGULAR_TILE: @@ -1455,17 +1505,33 @@ static void mkv_write_video_projection(AVFormatContext *s, EbmlWriter *writer, av_assert0(0); } - if (spherical->yaw) - ebml_writer_add_float(writer, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, - (double) spherical->yaw / (1 << 16)); - if (spherical->pitch) - ebml_writer_add_float(writer, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, - (double) spherical->pitch / (1 << 16)); - if (spherical->roll) - ebml_writer_add_float(writer, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, - (double) spherical->roll / (1 << 16)); + *yaw = (double) spherical->yaw / (1 << 16); + *pitch = (double) spherical->pitch / (1 << 16); + *roll = (double) spherical->roll / (1 << 16); - ebml_writer_close_master(writer); + return 1; /* Projection included */ +} + +static void mkv_write_video_projection(void *logctx, EbmlWriter *wr, + const AVStream *st, uint8_t private[]) +{ + double yaw = 0, pitch = 0, roll = 0; + int ret; + + ebml_writer_open_master(wr, MATROSKA_ID_VIDEOPROJECTION); + + ret = mkv_handle_spherical(logctx, wr, st, private, &yaw, &pitch, &roll); + if (!ret) + mkv_handle_rotation(logctx, st, &yaw, &roll); + + if (yaw) + ebml_writer_add_float(wr, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, yaw); + if (pitch) + ebml_writer_add_float(wr, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, pitch); + if (roll) + ebml_writer_add_float(wr, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, roll); + + ebml_writer_close_or_discard_master(wr); } #define MAX_FIELD_ORDER_ELEMS 2 diff --git a/tests/ref/fate/matroska-dovi-write-config8 b/tests/ref/fate/matroska-dovi-write-config8 index bb22563eee..58eb454865 100644 --- a/tests/ref/fate/matroska-dovi-write-config8 +++ b/tests/ref/fate/matroska-dovi-write-config8 @@ -1,5 +1,5 @@ -09ff3c0a038eec0cdf4773929b24f41a *tests/data/fate/matroska-dovi-write-config8.matroska -3600606 tests/data/fate/matroska-dovi-write-config8.matroska +80d2b23a6f27ab28b02a907b37b9649c *tests/data/fate/matroska-dovi-write-config8.matroska +3600620 tests/data/fate/matroska-dovi-write-config8.matroska #extradata 0: 551, 0xa18acf66 #extradata 1: 2, 0x00340022 #tb 0: 1/1000 @@ -46,6 +46,15 @@ 1, 395, 395, 23, 439, 0x7d85e4c9 [STREAM] [SIDE_DATA] +side_data_type=Display Matrix +displaymatrix= +00000000: 0 65536 0 +00000001: -65536 0 0 +00000002: 0 0 1073741824 + +rotation=-90 +[/SIDE_DATA] +[SIDE_DATA] side_data_type=DOVI configuration record dv_version_major=1 dv_version_minor=0
Matroska supports orthogonal transformations (both pure rotations as well as reflections) via its 3D-projection elements, namely ProjectionPoseYaw (for a horizontal reflection) as well as ProjectionPoseRoll (for rotations). This commit adds support for this. Support for this in the demuxer has been added in 937bb6bbc1e8654633737e69e403e95a37113058 and the sample used in the matroska-dovi-write-config8 FATE-test includes a displaymatrix indicating a rotation which is now properly written and read, thereby providing coverage for the relevant code in the muxer as well as the demuxer. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Honestly, I am not really sure how to handle the floating-point inaccuracies here (in atan2). libavformat/matroskaenc.c | 100 +++++++++++++++++---- tests/ref/fate/matroska-dovi-write-config8 | 13 ++- 2 files changed, 94 insertions(+), 19 deletions(-)