From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 85E714AD90 for ; Sat, 22 Jun 2024 20:19:57 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id ED37868D4C2; Sat, 22 Jun 2024 23:19:53 +0300 (EEST) Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id F062468D01E for ; Sat, 22 Jun 2024 23:19:46 +0300 (EEST) Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-706642b4403so565756b3a.1 for ; Sat, 22 Jun 2024 13:19:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719087584; x=1719692384; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=bQq5RviwX+lr0ENLzpbbUHSoTKCS28eroawzIkpKBUQ=; b=auro8XzvVdpYIqrGVac20pAijz1p33e12cGJhCjuF+wmyPVEClDEw3JWiIR3CgN1Iz QmoOBmR47VF2DxCWHKJoFtbnYb8PKyhUcvb0Z8i7EzdsZcFOOaObg8pdZTNn2kFNHu98 /9r1IOzTK2f0yBQZM6HV7RCaPW3CvYAPHqb7w2+dTm7Ct7o+sPeEvrKhkMEnR61asJCG hEOdWcGendHrG4s2waazej8O33dob/VVaE7QqEpZZ5FsAzl4DqjpoAwsQn3nCdtafLHp F5G+jLDfVm3Xv9+vG6UW/Ef3Kpztaho7kesGoXwmSNqb8bfn9oO1yADnOQ2Nfb3O/hf4 Ai1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719087584; x=1719692384; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bQq5RviwX+lr0ENLzpbbUHSoTKCS28eroawzIkpKBUQ=; b=AZxMXVJYPXAGIIb5lEnsCWcN7q7NGyk+1Jpv74VIPC5vLu3HKLzz/YlKV2w4TMoSX4 NIzFUPz+Y5+Qm346c/eWIA00RVcySLtkQIF32pUWp8RwV910EOgs+xiOJU63beuFyKUt yI7o/pM4FgVB7TRJkm+PgLjjCeTyxLbBAijDGzWJum0IiDze3YOdQN9G8rIYe1nF2OLy 8pp8p5Dlxu0ftNsUCAwlZEbOFMgtTEckXGLTx5l2Xcq9QRFIlQyZEf7e7BWT2ik2QZ7M g+oh3fIS88WTW7tNJqiYerzBPUQ1Bb3nI1xHD//W39xcdjGHsYLl7yPISboPJdICy8yk e65g== X-Gm-Message-State: AOJu0YyyJRsLc2WFgE9i0NWPTqpZO2P+PvPoNbJCoCgw833WXEJfMo79 nOMkEYN31lL2ENb056kxLXawDJ2W4EENOcy7HXlmv+JWGuUZPZEsgqlZUA== X-Google-Smtp-Source: AGHT+IGUJ96610A6Y2+4iHLN5My6SX2s1cgXS0/Ejfb0g50c4LmQUsDzcp4IY5Lbw+gU20O09Vmz3Q== X-Received: by 2002:a05:6a20:f38a:b0:1bc:fd17:b922 with SMTP id adf61e73a8af0-1bcfd17bbe4mr36097637.37.1719087583444; Sat, 22 Jun 2024 13:19:43 -0700 (PDT) Received: from [192.168.0.16] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7065ca6a47asm2559326b3a.66.2024.06.22.13.19.42 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 22 Jun 2024 13:19:42 -0700 (PDT) Message-ID: <279f38ed-f2ce-4fe0-98f4-09495dd97bb3@gmail.com> Date: Sat, 22 Jun 2024 17:19:54 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240622201116.403924-1-dev@lynne.ee> Content-Language: en-US From: James Almer In-Reply-To: <20240622201116.403924-1-dev@lynne.ee> Subject: Re: [FFmpeg-devel] [PATCH] lavu/stereo3d: change baseline and horizontal FOV to rationals X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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. 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? _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".