On 6/22/2024 6:26 PM, Lynne via ffmpeg-devel wrote:
On 22/06/2024 22:19, James Almer wrote:
On 6/22/2024 5:11 PM, Lynne via ffmpeg-devel wrote:
This avoids hardcoding any implementation-specific limitiations as
part of the API, and allows for future expandability.

This also allows API users to more conveniently convert the
values into floats without hardcoding specific conversion constants.

The draft implementation of this mechanism for the draft of
AVTransport uses rationals for these particular fields.

The API was committed 2 days ago, so changing these fields now
is within the realms of acceptable.
---
  fftools/ffprobe.c                            |  4 ++--
  libavformat/dump.c                           |  9 +++++----
  libavformat/mov.c                            |  9 ++++++---
  libavutil/stereo3d.h                         |  8 ++++----
  tests/ref/fate/matroska-spherical-mono       |  4 ++--
  tests/ref/fate/matroska-spherical-mono-remux |  8 ++++----
  tests/ref/fate/matroska-stereo_mode          | 16 ++++++++--------
  tests/ref/fate/matroska-vp8-alpha-remux      |  4 ++--
  tests/ref/fate/mov-spherical-mono            |  4 ++--
  9 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index d7ba980ff9..2da928b7c3 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2546,9 +2546,9 @@ static void print_pkt_side_data(WriterContext *w,
              print_int("inverted", !!(stereo->flags & AV_STEREO3D_FLAG_INVERT));
              print_str("view", av_stereo3d_view_name(stereo->view));
              print_str("primary_eye", av_stereo3d_primary_eye_name(stereo->primary_eye));
-            print_int("baseline", stereo->baseline);
+            print_q("baseline", stereo->baseline, '/');
              print_q("horizontal_disparity_adjustment", stereo- >horizontal_disparity_adjustment, '/'); -            print_int("horizontal_field_of_view", stereo- >horizontal_field_of_view); +            print_q("horizontal_field_of_view", stereo- >horizontal_field_of_view, '/');
          } else if (sd->type == AV_PKT_DATA_SPHERICAL) {
              const AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data;               print_str("projection", av_spherical_projection_name(spherical->projection));
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 61a2c6a29f..ac7d3038ab 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -262,13 +262,14 @@ static void dump_stereo3d(void *ctx, const AVPacketSideData *sd, int log_level)
      av_log(ctx, log_level, "%s, view: %s, primary eye: %s",
             av_stereo3d_type_name(stereo->type), av_stereo3d_view_name(stereo->view),
             av_stereo3d_primary_eye_name(stereo->primary_eye));
-    if (stereo->baseline)
-        av_log(ctx, log_level, ", baseline: %"PRIu32"", stereo- >baseline);
+    if (stereo->baseline.num && stereo->baseline.den)
+        av_log(ctx, log_level, ", baseline: %d/%d", stereo- >baseline.num, stereo->baseline.den);       if (stereo->horizontal_disparity_adjustment.num && stereo- >horizontal_disparity_adjustment.den)           av_log(ctx, log_level, ", horizontal_disparity_adjustment: %d/%d",                  stereo->horizontal_disparity_adjustment.num, stereo- >horizontal_disparity_adjustment.den);
-    if (stereo->horizontal_field_of_view)
-        av_log(ctx, log_level, ", horizontal_field_of_view: %"PRIu32"", stereo->horizontal_field_of_view); +    if (stereo->horizontal_field_of_view.num && stereo- >horizontal_field_of_view.den) +        av_log(ctx, log_level, ", horizontal_field_of_view: %d/%d", stereo->horizontal_field_of_view.num,
+               stereo->horizontal_field_of_view.den);
      if (stereo->flags & AV_STEREO3D_FLAG_INVERT)
          av_log(ctx, log_level, " (inverted)");
diff --git a/libavformat/mov.c b/libavformat/mov.c
index f08fec3fb6..64cd17e770 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6545,7 +6545,8 @@ static int mov_read_eyes(MOVContext *c, AVIOContext *pb, MOVAtom atom)
      MOVStreamContext *sc;
      int size, flags = 0;
      int64_t remaining;
-    uint32_t tag, baseline = 0;
+    uint32_t tag;
+    AVRational baseline = av_make_q(0, 0);
      enum AVStereo3DView view = AV_STEREO3D_VIEW_PACKED;
      enum AVStereo3DPrimaryEye primary_eye = AV_PRIMARY_EYE_NONE;
      AVRational horizontal_disparity_adjustment = { 0, 1 };
@@ -6642,7 +6643,8 @@ static int mov_read_eyes(MOVContext *c, AVIOContext *pb, MOVAtom atom)
              avio_skip(pb, 1); // version
              avio_skip(pb, 3); // flags
-            baseline = avio_rb32(pb);
+            baseline.num = avio_rb32(pb);
+            baseline.den = 1000000; // micrometers
              break;
          }
@@ -6782,7 +6784,8 @@ static int mov_read_hfov(MOVContext *c, AVIOContext *pb, MOVAtom atom)
              return AVERROR(ENOMEM);
      }
-    sc->stereo3d->horizontal_field_of_view = avio_rb32(pb);
+    sc->stereo3d->horizontal_field_of_view.num = avio_rb32(pb);
+    sc->stereo3d->horizontal_field_of_view.den = 1000; // thousands of a degree
      return 0;
  }
diff --git a/libavutil/stereo3d.h b/libavutil/stereo3d.h
index 00a5c3900e..097f6c9455 100644
--- a/libavutil/stereo3d.h
+++ b/libavutil/stereo3d.h
@@ -213,9 +213,9 @@ typedef struct AVStereo3D {
      /**
       * The distance between the centres of the lenses of the camera system,
-     * in micrometers. Zero if unset.
+     * in meters. Zero if unset.
       */
-    uint32_t baseline;
+    AVRational baseline;

This is not ok. No reason for distance be a soft float, or allow negative values. In the case of the blin box, it's a 32bit unsigned value, which will not fit in AVRational.

Added an (int32_t) cast locally. This is also how horizontal_disparity_adjustment is handled, with a uint32_t converted into a rational.

No, that one is an int32_t as it can be negative (Ranges -1.0 to 1.0), thus it works fine as AVRational. baseline does not, so it should stay an uint32_t.



You can easily convert this in your code by using a 1000000 denominator if you wish.

      /**
       * Relative shift of the left and right images, which changes the zero parallax plane.
@@ -224,9 +224,9 @@ typedef struct AVStereo3D {
      AVRational horizontal_disparity_adjustment;
      /**
-     * Horizontal field of view in thousanths of a degree. Zero if unset.
+     * Horizontal field of view, in degrees. Zero if unset.
       */
-    uint32_t horizontal_field_of_view;
+    AVRational horizontal_field_of_view;

This one i can't say. Would for example Matroska implement degrees as a float, or uint?

Likely a float (EBML_FLOAT). This is how mastering metadata and video projection are handled.

But Mastering metadata is not in degrees.

Wait for Derek to comment. He knows this field better than me.


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to