From patchwork Sun Oct 16 13:28:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 1017 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.140.133 with SMTP id o127csp5392vsd; Sun, 16 Oct 2016 06:28:25 -0700 (PDT) X-Received: by 10.194.85.229 with SMTP id k5mr8410498wjz.22.1476624505207; Sun, 16 Oct 2016 06:28:25 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id l10si17016870wjk.232.2016.10.16.06.28.24; Sun, 16 Oct 2016 06:28:25 -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=@gmail.com; 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; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 07B9F689B0F; Sun, 16 Oct 2016 16:28:20 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-yb0-f180.google.com (mail-yb0-f180.google.com [209.85.213.180]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C339D68971F for ; Sun, 16 Oct 2016 16:28:13 +0300 (EEST) Received: by mail-yb0-f180.google.com with SMTP id x128so2026879ybg.1 for ; Sun, 16 Oct 2016 06:28:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to; bh=Nf3jODN3BFGrV69/mBJbvptDWWsFCgDAkNfd53+uxls=; b=xuGkb8zMZ2ut3xqKrlBk5TyvPBDHbkrGERFLgrr39ZDMKUkDaw8305v34CeaWG6mJU U0ZasdFqActADCGL9wpwtABx8LlhQ4mP0xoCG/4xWTj7EqvnY1/GqD3fQ+E31X444Vob 1yv0F6uPZ0KbyhFVxzL3G7k22zaTHVQfeU6J9I+I2hxMn0XEAjt9gLtSCSik6gXSx0c4 d5MNTWO/8aqYzYiokBswAKwOH1jBvL0TXsZa8BhDv0lL3Ifz5gsifyDaoG6tbIjjGl8r sxR2nadhOsePVjRK4GWhTTgFeifAQkLdsJgZyfHmy4YB+eg5us4OQ3sMrCKAFrmylVz+ ZuIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to; bh=Nf3jODN3BFGrV69/mBJbvptDWWsFCgDAkNfd53+uxls=; b=fsdayprS0q5yhlg+GiXpoV9jA65N6t/DEQS54WK8PdMpdP23Z9179745BxZ0sRqU2z ip6EsI/aRuQZqui3tDEcR/NyMJoXm7cU7ZktJ87mziVpA06IFj9fqOJ2kTRiDfqDNbBi fB1gga8WkOCYDkKkLsEPctLg7FyZSA8d8dlBWiJaSUPVScxfA8kU9DixUgPXgJuJtS4M bkvtxNT1TS0y9Elq4jlOINUr+ayESeMT7RGvpCpjbRtmsAMt9ujnHyZ3YL3Xtl/VBhiF aWlgxTVb5TLbsERExQCbkrOh5mhVsNp3ev1uKtDF9hSG+CyJnbMoDcSwgSQyqO/Ir8pE gzUw== X-Gm-Message-State: AA6/9RkdFpMmW/Nule58C+52BEjAVQGdQZDzywxbsDi6KM8tVmpkhELl5dYPIYrejjJzGA== X-Received: by 10.37.171.71 with SMTP id u65mr19596800ybi.128.1476624493652; Sun, 16 Oct 2016 06:28:13 -0700 (PDT) Received: from [192.168.1.33] ([181.22.59.227]) by smtp.googlemail.com with ESMTPSA id o125sm9973419ywe.31.2016.10.16.06.28.12 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 16 Oct 2016 06:28:12 -0700 (PDT) To: FFmpeg development discussions and patches References: <20161015220943.660-1-jamrial@gmail.com> <20161016085813.GA4013338@phare.normalesup.org> From: James Almer Message-ID: Date: Sun, 16 Oct 2016 10:28:06 -0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161016085813.GA4013338@phare.normalesup.org> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avformat/matroskadec: set aspect ratio only when DisplayWidth and DisplayHeight are in pixels X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 10/16/2016 5:58 AM, Nicolas George wrote: > Le quartidi 24 vendémiaire, an CCXXV, James Almer a écrit : >> A missing DisplayUnit element or one with the default value of 0 means >> DisplayWidth and DisplayHeight should be interpreted as pixels. >> >> The current code setting st->sample_aspect_ratio is wrong when DisplayUnit >> is anything else. > > Sorry to react after it was pushed, but: are you sure about the logic? > Naively, I think that a/b makes sense whatever the unit for a and b, as long > as it is known and the same: the logic should be applied for all units > except UNKNOWN. What am I missing? Nothing probably, just me not giving it much thought after i couldn't find any sample using cm, in or dar instead of pixels, and assuming the calculations were tailored specifically for sizes in pixels. I manually created some and you're right it seems to work fine with all of them. Is the attached patch ok? From 14a71576f69a7fdb4fd48502ec2ea36c26297004 Mon Sep 17 00:00:00 2001 From: James Almer Date: Sun, 16 Oct 2016 10:13:45 -0300 Subject: [PATCH] Partially revert "avformat/matroskadec: set aspect ratio only when DisplayWidth and DisplayHeight are in pixels" The code works just fine regardless of unit, so only make sure DisplayUnit is not "unknown". Found-by: Nicolas George Signed-off-by: James Almer --- libavformat/matroskadec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 0d17a7e..f9f54ff 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -2297,7 +2297,7 @@ static int matroska_parse_tracks(AVFormatContext *s) if (track->video.stereo_mode && track->video.stereo_mode < MATROSKA_VIDEO_STEREOMODE_TYPE_NB) mkv_stereo_mode_display_mul(track->video.stereo_mode, &display_width_mul, &display_height_mul); - if (track->video.display_unit == MATROSKA_VIDEO_DISPLAYUNIT_PIXELS) { + if (track->video.display_unit != MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) { av_reduce(&st->sample_aspect_ratio.num, &st->sample_aspect_ratio.den, st->codecpar->height * track->video.display_width * display_width_mul, -- 2.9.1