From patchwork Wed Nov 14 20:51:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Niedermayer X-Patchwork-Id: 11024 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 112B644C96D for ; Wed, 14 Nov 2018 22:51:54 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DC5EE680D3A; Wed, 14 Nov 2018 22:51:15 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id AC0D1680CAD for ; Wed, 14 Nov 2018 22:51:09 +0200 (EET) X-Originating-IP: 213.47.41.20 Received: from localhost (213-47-41-20.cable.dynamic.surfer.at [213.47.41.20]) (Authenticated sender: michael@niedermayer.cc) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id D4BB940009 for ; Wed, 14 Nov 2018 20:51:46 +0000 (UTC) Date: Wed, 14 Nov 2018 21:51:45 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20181114205145.GU3343@michaelspb> References: MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: [FFmpeg-devel] [PATCH] avcodec/vc1: correct aspect ratio calculation 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" Hi On Wed, Nov 14, 2018 at 10:57:25AM +0100, Jerome Borsboom wrote: > According to VC-1 spec: > * Display size defaults to max coded size when not explicitly set in > sequence header > * Aspect ratio in the sequence header refers to the Display size elements. > Therefore, the aspect ratio for the coded samples (SAR) needs to take into > account the scaling from coded size to display size, and the aspect ratio > of the display size elements. > > Signed-off-by: Jerome Borsboom > --- > VC-1 spec assumes that the output of the decoder, i.e. the pixel matrix with > dimensions coded_width x coded_height, is scaled to display size, i.e. a pixel > matrix of display_horiz_size x display_vert_size. This may introduce part of > the sample aspect ratio when the sizes of the two pixel matrices are not equal. > A further part of the sample aspect ratio is optionally specified in the sequence > header as the aspect ratio of the display size pixels. This patch takes both > aspect ratios into account and aims to be correct even when the coded image > includes overscan regions. > > libavcodec/vc1.c | 38 +++++++++++++++++++++----------------- > libavcodec/vc1.h | 2 ++ > 2 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c > index 3581d87b57..efc6edc4b0 100644 > --- a/libavcodec/vc1.c > +++ b/libavcodec/vc1.c > @@ -442,30 +442,24 @@ static int decode_sequence_header_adv(VC1Context *v, GetBitContext *gb) > } > v->s.max_b_frames = v->s.avctx->max_b_frames = 7; > if (get_bits1(gb)) { //Display Info - decoding is not affected by it > - int w, h, ar = 0; > + int ar = 0; > av_log(v->s.avctx, AV_LOG_DEBUG, "Display extended info:\n"); > - w = get_bits(gb, 14) + 1; > - h = get_bits(gb, 14) + 1; > - av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n", w, h); > + v->disp_horiz_size = get_bits(gb, 14) + 1; > + v->disp_vert_size = get_bits(gb, 14) + 1; > + av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n", > + v->disp_horiz_size, v->disp_vert_size); > if (get_bits1(gb)) > ar = get_bits(gb, 4); > if (ar && ar < 14) { > - v->s.avctx->sample_aspect_ratio = ff_vc1_pixel_aspect[ar]; > + v->aspect_ratio = ff_vc1_pixel_aspect[ar]; > } else if (ar == 15) { > - w = get_bits(gb, 8) + 1; > - h = get_bits(gb, 8) + 1; > - v->s.avctx->sample_aspect_ratio = (AVRational){w, h}; > + v->aspect_ratio = (AVRational){get_bits(gb, 8) + 1, get_bits(gb, 8) + 1}; iam not sure this is valid C and not undefined but either way this patch breaks fate TEST vc1-ism Test vc1-ism failed. Look at tests/data/fate/vc1-ism.err for details. make: *** [fate-vc1-ism] Error 1 [...] --- ./tests/ref/fate/vc1-ism 2018-11-13 19:52:23.489023763 +0100 +++ tests/data/fate/vc1-ism 2018-11-14 21:50:11.522992878 +0100 @@ -2,7 +2,7 @@ #media_type 0: video #codec_id 0: rawvideo #dimensions 0: 240x104 -#sar 0: 156/156 +#sar 0: 13/30 0, 0, 0, 1, 37440, 0xd1bc5235 0, 2, 2, 1, 37440, 0x158e6167 0, 3, 3, 1, 37440, 0x0faa4481