From patchwork Sat May 25 16:18:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Niklas Haas X-Patchwork-Id: 49259 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a59:542:0:b0:460:55fa:d5ed with SMTP id 63csp2388388vqf; Sat, 25 May 2024 09:18:53 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCXGPVSnpsIJVSnPMjdRHP45EZRpArNABqqT9juM4+OwrqdChsSvMK+r6X6KhOXKR5m253AZ5bhBOSrcSGOW5G9zlkhm1saxXlUSqA== X-Google-Smtp-Source: AGHT+IGdHE36gdeTVUxBFkDdmdSjIvbtr/HRlGxrRZ0D18ITzuMV8sakbQCGZkTgZlAxwC0cuC1r X-Received: by 2002:a50:a45d:0:b0:578:632e:6794 with SMTP id 4fb4d7f45d1cf-578632e6d9fmr3156010a12.10.1716653933243; Sat, 25 May 2024 09:18:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1716653933; cv=none; d=google.com; s=arc-20160816; b=PiuiZKb3lppxGE3146UdXal2vx/3bIRIMKNOjTZdBCOon6S+m8ny8PNOs5KoHBqwgz +dO9XTz0cuHqybdExXesSh4jaPEqTOr8NEuhnpZAZZ5I9T5MV0xJuUm606DkrlA0gVji IR8OTCrx1xYKSmXq7ey/P4mDDyB5LBuEBUYAu5D0h81/9vQ1+dJ2cbmFeTGHYbR39eJT Xbncycy3JI9YDR2MNywP/P5QPpkHD6ORMvbcVMqS+wOAAXE7DDB3S0osZb87+kpTHZCF 9rllu6pArEA/6ECn19WJVXQsniJ5FxuRo9BsL1SNhax9qJ3wgqZlj+ZGr2v8uC1Ol7nI X+Vw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:mime-version:message-id:date:to:from :dkim-signature:delivered-to; bh=Yf7qUAxOvMmxciJcnJXTs9UBTA8SbxidIkFPRatrCLA=; fh=xmAeKtysnShNOmkhiJmYkS30uw4Fu2hvBJ7qlIwukxQ=; b=ifoyORSwzX/JZlxre3v38p++IFQuijm5Q5sV23s7a36Uq0DYKidOsKncQeoZ1Bui00 c6tkWCDiRDOtcJShGbWH5JEmAlidUNnL3wWDHaPOX80lJrIuH2MgsEotA1paJ0Z5XFN/ Qbl0T3PO2pFqoi72pPogaB5atJrAeMiL8G24VYf0Oxl3YhWSzYCMAcXdrKU/HNwjVJog Y5q4N6s9LDZbBs66wVx+qVYDX4b+7rBGNHtRcGrAJWAMwaRNPAltr9OQ/gEOgbeWciXp V+VQvl4M0zRHakxqx+tdSQFgR/fzPadn6MJoTejb2887VCHdQYdqbDGCRmVPnMyIizGY jGeA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@haasn.xyz header.s=mail header.b=Z2hN46T8; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id 4fb4d7f45d1cf-5786385626dsi524620a12.202.2024.05.25.09.18.52; Sat, 25 May 2024 09:18:53 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@haasn.xyz header.s=mail header.b=Z2hN46T8; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 381AE68D565; Sat, 25 May 2024 19:18:49 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2BBFF68D288 for ; Sat, 25 May 2024 19:18:42 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1716653921; bh=DsKiazwkZi7+Do0BlDRILBHDKbLf2hvwPFGojPcacsQ=; h=From:To:Cc:Subject:Date:From; b=Z2hN46T8StwNJG2gGAm723Vx2aQ5mmW/A0b2flr9fQPiWv7gOrQV0RRTePjIL+SR7 O+jcBGAJdTJE6Da5WAcT8ojg4YKTbILwe6dleoNHv9sE78mrJ+p2HeyjcazEOzyZaq sgJDLyoY+liKX5ifUU3cjNmoJzyflpjtMATLGD44= Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id DAC16406AD; Sat, 25 May 2024 18:18:41 +0200 (CEST) From: Niklas Haas To: ffmpeg-devel@ffmpeg.org Date: Sat, 25 May 2024 18:18:39 +0200 Message-ID: <20240525161839.34867-1-ffmpeg@haasn.xyz> X-Mailer: git-send-email 2.45.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avdovi/dovi_rpu{enc, dec}: fix ms_weight handling 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 Cc: Niklas Haas Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: WxZuB46XT5Hs From: Niklas Haas The code as written was wrong. The l2.ms_weight value is coded as a *signed* integer, rather than a shifted unsigned integer. (And even so, the offset of 8192 would be too large, since 2^12 = 4096. Ditto for l8.ms_weight, which should have an offset of 2048, not 8192) In addition, because the l8.ms_weight struct member is unsigned, these shifting semantics ended up overflowing the field, leading to undefined behavior when transcoding. Fortunately, the damage was relatively contained in practice, because it just corrupts the coding of this field, which is ignored in practice in all implementations I have seen. For some reason, Dolby decided to add a sign bit to l2.ms_weight but not l3.ms_weight. In either case, it's quite probably that this is still a shifted unsigned integer in disguise, since all samples seem to have a value of 2048 for these fields. But without specs, that's guesswork, and the official DV validator also reports these fields as unsigned integers with a value of 2048. --- libavcodec/dovi_rpudec.c | 4 ++-- libavcodec/dovi_rpuenc.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c index 7c7eda9d09..a477dbd4e3 100644 --- a/libavcodec/dovi_rpudec.c +++ b/libavcodec/dovi_rpudec.c @@ -142,7 +142,7 @@ static void parse_ext_v1(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm) dm->l2.trim_power = get_bits(gb, 12); dm->l2.trim_chroma_weight = get_bits(gb, 12); dm->l2.trim_saturation_gain = get_bits(gb, 12); - dm->l2.ms_weight = get_bits(gb, 13) - 8192; + dm->l2.ms_weight = get_sbits(gb, 13); break; case 4: dm->l4.anchor_pq = get_bits(gb, 12); @@ -197,7 +197,7 @@ static void parse_ext_v2(DOVIContext *s, GetBitContext *gb, AVDOVIDmData *dm, dm->l8.trim_power = get_bits(gb, 12); dm->l8.trim_chroma_weight = get_bits(gb, 12); dm->l8.trim_saturation_gain = get_bits(gb, 12); - dm->l8.ms_weight = get_bits(gb, 12) - 8192; + dm->l8.ms_weight = get_bits(gb, 12); if (ext_block_length < 12) break; dm->l8.target_mid_contrast = get_bits(gb, 12); diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c index 3c3e0f84c0..dacb8b54e7 100644 --- a/libavcodec/dovi_rpuenc.c +++ b/libavcodec/dovi_rpuenc.c @@ -276,7 +276,7 @@ static void generate_ext_v1(PutBitContext *pb, const AVDOVIDmData *dm) put_bits(pb, 12, dm->l2.trim_power); put_bits(pb, 12, dm->l2.trim_chroma_weight); put_bits(pb, 12, dm->l2.trim_saturation_gain); - put_bits(pb, 13, dm->l2.ms_weight + 8192); + put_sbits(pb, 13, dm->l2.ms_weight); break; case 4: put_bits(pb, 12, dm->l4.anchor_pq); @@ -374,7 +374,7 @@ static void generate_ext_v2(PutBitContext *pb, const AVDOVIDmData *dm) put_bits(pb, 12, dm->l8.trim_power); put_bits(pb, 12, dm->l8.trim_chroma_weight); put_bits(pb, 12, dm->l8.trim_saturation_gain); - put_bits(pb, 12, dm->l8.ms_weight + 8192); + put_bits(pb, 12, dm->l8.ms_weight); if (ext_block_length < 12) break; put_bits(pb, 12, dm->l8.target_mid_contrast);