From patchwork Tue Oct 11 08:29:07 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carl Eugen Hoyos X-Patchwork-Id: 951 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.140.66 with SMTP id o63csp219511vsd; Tue, 11 Oct 2016 01:29:18 -0700 (PDT) X-Received: by 10.194.17.71 with SMTP id m7mr3288667wjd.179.1476174558219; Tue, 11 Oct 2016 01:29:18 -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 me20si3385650wjb.290.2016.10.11.01.29.17; Tue, 11 Oct 2016 01:29:18 -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 1A79B68991D; Tue, 11 Oct 2016 11:29:16 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-io0-f179.google.com (mail-io0-f179.google.com [209.85.223.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7282C689886 for ; Tue, 11 Oct 2016 11:29:09 +0300 (EEST) Received: by mail-io0-f179.google.com with SMTP id r30so17659105ioi.1 for ; Tue, 11 Oct 2016 01:29:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=2RJ8u7liiSKyeYfQY4AiSamsgp6GJP91fBBfYMLW1lw=; b=mzESNtrHh8BfvEPS0Ljd1WSxDAfDO+KVas5IfpDzNPcvWREd/CGPWwOLA7byCgzvZs +eVycSIPD7jCTlPjsXUMvoJGMA41TqCiptKZGM9/e+5q2JamEqbAMpIYiCH2UVNlgf1A uu/Ybv60QhSwLdSTa71xD0zgXOiHBXtRPe7MSNJS47rLaYpf98dgeCH/ztxaH70EqIzM a8kgKA3ycH5cefmjB/+v9XakNK9dmziWY1zGAaTs5qN2FliPeu7EcT13YaSMyusia32Y jZeRGj28dGMLj/dIhfVIOxBatQ/vPrhkjnIrgSsc0WJPdtAAE+5PTP+adE/ekym0w2xF fV9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=2RJ8u7liiSKyeYfQY4AiSamsgp6GJP91fBBfYMLW1lw=; b=Q6Y2kYtPGI9DEFmmRe8eFbs7YKuYHv+gxEd9gPikxhUcTvAbhQpq2CTFO5HRrPK7As ZwCEx3pyEcTC4jbJCunMTdyuJfz3Zgpd459ehIOpFNuWD3R3BJmA6Y/1fd6oDLm+Rx3/ OucR9s6JdJzBjUiKlP4ZFNsLQRFvlVxZSRv+DaPBVCkxNn4wdsCMt7Gwcu+c1lshtuZI CD1W0xm8Bya07IT71ejr6N8LUre4Ulm2iTr3tiEeV7pC02EaRMmf71x8Yp0s8BM04uU9 k/WeOYzKT4XnvuaeBFZ9zezrSxnwh8r4CbsK7B2LfzYXwFXZSo2xxnx+EeAHVMTYwqNB BOXA== X-Gm-Message-State: AA6/9Rl9ki6EjlckHn7QRupOO9/AqZqYlU2eKtOrXLIxV7/9oR6wjz1K1Voq+TvPwgLA6A7lTOC5+4Hu0Wmwmw== X-Received: by 10.107.155.200 with SMTP id d191mr4818498ioe.64.1476174547851; Tue, 11 Oct 2016 01:29:07 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.38.150 with HTTP; Tue, 11 Oct 2016 01:29:07 -0700 (PDT) In-Reply-To: References: <201608281324.30747.cehoyos@ag.or.at> From: Carl Eugen Hoyos Date: Tue, 11 Oct 2016 10:29:07 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [RFC/PATCH]lavc/vp8: Read a frame to set pix_fmt 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! Sorry for the delay! 2016-08-28 14:25 GMT+02:00 Ronald S. Bultje : > On Sun, Aug 28, 2016 at 7:24 AM, Carl Eugen Hoyos wrote: >> Currently, FFmpeg reports "1920x1080" for >> fate-suite/vp8/frame_size_change.webm although the first >> frame is 160x90. >> This is different for "--enable-libvpx --disable-decoder=vp8" >> and would be different if the sample used vp9 or h264 or hevc. >> I believe this will be different once the native vp8 decoder >> supports transparency. >> So I think attached patch is not unreasonable. >> Strong objections? >> >> The sample from #5803 is broken and should not be used in the >> reasoning. > This is (IMO) mostly a terminology issue, no? > > From what I recall (you didn't mention), the issue in the size-change > sample is that the demuxer reads size (AVCodecContext->width/height) from > container (which is 1920x1080), and if decoders sets > (AVCodecContext->)pix_fmt, this satisfies avformat_find_stream_info to not > decode a frame before returning. With your change, AVCodecContext->pix_fmt > is missing and thus the size (along with pix_fmt) is read from the first > frame (160x90). This size is then used by ffmpeg.c to set (default) output > size. Which of these two situations is better? I don't know. It's a > terminology issue. I don't know if there is a correct answer. I agree, I am just arguing that vp8 now acts differently from very similar codecs (vp9) and from vp8 in the future and I think it is ok to change this now (since a user requested it). > Answer me this question (without caring for what libvpx or h264 or vp8 does > right now): if a container says size=AxB for a C-frame sample with first > frame size=DxE and unknown size for any subsequent frame, and A-E are all > integers > 0 (and C > 1, and A!=D||B!=E), what should the (default) output > size be? Why? I don't think there is a general answer. Although arguing that the container resolution should be preferred for displaying makes sense but not applying this patch does not fix the issue that FFmpeg currently does not use container resolution independently from codec resolution. > (The patch itself is probably fine.) New patch attached, the original one was broken after too much testing and changing. Carl Eugen From 7e9ffdb9e4385bc8c33e063266d282dc9125a6e7 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos Date: Tue, 11 Oct 2016 10:27:51 +0200 Subject: [PATCH] lavc/vp8: Read a frame to set pix_fmt. This is what libvpx does and what is done for other codecs with similar use-cases and what will have to be done for VP8A. --- libavcodec/vp8.c | 3 +- libavcodec/webp.c | 1 + tests/ref/fate/vp8-size-change | 62 ++++++++++++++++++++-------------------- 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c index c1c3eb7..5683493 100644 --- a/libavcodec/vp8.c +++ b/libavcodec/vp8.c @@ -2585,6 +2585,8 @@ int vp78_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, ret = AVERROR_INVALIDDATA; goto err; } + if (avctx->pix_fmt != AV_PIX_FMT_YUVA420P) + avctx->pix_fmt = AV_PIX_FMT_YUV420P; curframe->tf.f->key_frame = s->keyframe; curframe->tf.f->pict_type = s->keyframe ? AV_PICTURE_TYPE_I @@ -2728,7 +2730,6 @@ int vp78_decode_init(AVCodecContext *avctx, int is_vp7) s->avctx = avctx; s->vp7 = avctx->codec->id == AV_CODEC_ID_VP7; - avctx->pix_fmt = AV_PIX_FMT_YUV420P; avctx->internal->allocate_progress = 1; ff_videodsp_init(&s->vdsp, 8); diff --git a/libavcodec/webp.c b/libavcodec/webp.c index 45abfdc..723bfb7 100644 --- a/libavcodec/webp.c +++ b/libavcodec/webp.c @@ -1326,6 +1326,7 @@ static int vp8_lossy_decode_frame(AVCodecContext *avctx, AVFrame *p, if (!s->initialized) { ff_vp8_decode_init(avctx); + avctx->pix_fmt = AV_PIX_FMT_YUV420P; s->initialized = 1; if (s->has_alpha) avctx->pix_fmt = AV_PIX_FMT_YUVA420P; diff --git a/tests/ref/fate/vp8-size-change b/tests/ref/fate/vp8-size-change index 741b6d6..5105bc6 100644 --- a/tests/ref/fate/vp8-size-change +++ b/tests/ref/fate/vp8-size-change @@ -4,36 +4,36 @@ #tb 0: 1/30 #media_type 0: video #codec_id 0: rawvideo -#dimensions 0: 1920x1080 +#dimensions 0: 160x90 #sar 0: 1/1 #stream#, dts, pts, duration, size, hash -0, 0, 0, 1, 3110400, 7dde8cd136ab4b04a95d9856b941697e -0, 1, 1, 1, 3110400, aa885f78cb6374b5bfcc66a4fc57026f -0, 2, 2, 1, 3110400, b69b7b56f549a3f9b0a603940bac85ed -0, 3, 3, 1, 3110400, 20e2e0f0c89c58828b6a3b10d9e175e5 -0, 4, 4, 1, 3110400, 483997936e7d6bb849e64d50426ec689 -0, 5, 5, 1, 3110400, c85ef97a853ff594e2bfdf0a0a581dcc -0, 6, 6, 1, 3110400, c5e7b9ff4c25391793446da788cb83a9 -0, 7, 7, 1, 3110400, 63f93e89d24162e2f6328edbc6716b33 -0, 8, 8, 1, 3110400, 0e346ab4831ce8c69001153c72b7b827 -0, 9, 9, 1, 3110400, c526c21511d8bec2659d3d43d93734f2 -0, 10, 10, 1, 3110400, e95d01d5f9fb81a98bd34305c7ab30f8 -0, 11, 11, 1, 3110400, 177e75e7516e8746d31b43ea9d39e6b1 -0, 12, 12, 1, 3110400, 489d2bc0da93f118dc9a2697275697a7 -0, 13, 13, 1, 3110400, a2dc00d627350ff1ab302bcbad5ca5ac -0, 14, 14, 1, 3110400, 20ce143831b0189f763ee5bee9c51188 -0, 15, 15, 1, 3110400, 7822fd908bd81b521c23fa4a639caf9e -0, 16, 16, 1, 3110400, dabc4febbe09734126ac6f5a5180ba8c -0, 17, 17, 1, 3110400, ef88f0d6667feefac1471b065208e1c8 -0, 18, 18, 1, 3110400, 7c7fc665a6fd9e19af9358bbdc162a51 -0, 19, 19, 1, 3110400, f2bcf32f734f99506bdd0a0376badf82 -0, 20, 20, 1, 3110400, 06809c2d277fd3b3918ebb4b65c27661 -0, 21, 21, 1, 3110400, e403e9e86fa5d519f65c565b3add84b5 -0, 22, 22, 1, 3110400, d2b876730e12245cacb578307794349a -0, 23, 23, 1, 3110400, dfdfd8cb626a96138f6a2c1953dcf5ec -0, 24, 24, 1, 3110400, 0ac58c28575b804d9e63395653c3aef2 -0, 25, 25, 1, 3110400, 641f2a78e338c733ef159bd36ec7966f -0, 26, 26, 1, 3110400, 9402d455fa5bd556b85f479c42c3a4d2 -0, 27, 27, 1, 3110400, 0044d42b4048bc93112aa59789dbdc2d -0, 28, 28, 1, 3110400, 5d9e5c5ba35f6f452e5f31ccff9e819c -0, 29, 29, 1, 3110400, 307a55a94739b4cfdf41f7da7e5c0135 +0, 0, 0, 1, 21600, 5abd6c163522c7e882f7e9c369293bf9 +0, 1, 1, 1, 21600, 5c11d8cc9cc9102d0ef5afd1dc64aff1 +0, 2, 2, 1, 21600, cbeffa9ea9d682af77d3fd0fdf12c8c1 +0, 3, 3, 1, 21600, ea7cec515fcf8ccbc932d9e9b472cdc7 +0, 4, 4, 1, 21600, 23038b711dbac95ce710657b1fef5901 +0, 5, 5, 1, 21600, e0d6fb46bb5c0d939ee33af82b887668 +0, 6, 6, 1, 21600, 694518f14d3a2bd3c319bc0b098c78bb +0, 7, 7, 1, 21600, c1c7394bd4236afbc773af52ef7a10ea +0, 8, 8, 1, 21600, 4d8d3b2c9a637f963521585ea879357b +0, 9, 9, 1, 21600, b4444dc3cbf1b6cdd8047d3dcd497ffd +0, 10, 10, 1, 21600, 65e5d667ec9ceb636e21357f032ce800 +0, 11, 11, 1, 21600, fd9a4c67598051074387b640df7edaa9 +0, 12, 12, 1, 21600, 0e54e22d90f6296ae6989c83846272cd +0, 13, 13, 1, 21600, db4b1727450243b202bfec5ed6c73ae0 +0, 14, 14, 1, 21600, ab37a84be075ca42cc7351ff9fb1cb47 +0, 15, 15, 1, 21600, ae4d2d297e646bd8e05e76b457d9b576 +0, 16, 16, 1, 21600, e7cfd580e3c3d7c3f2f5136d1e548595 +0, 17, 17, 1, 21600, cbec09314a0b7ad53f4893eb474e1c65 +0, 18, 18, 1, 21600, e1fa89cd63c37496bc86f18694324d88 +0, 19, 19, 1, 21600, e9655b151253950313810228278ca104 +0, 20, 20, 1, 21600, 69ba31c0eff7bc93f4180173d8e64c60 +0, 21, 21, 1, 21600, 368a1f6a1172d7d56f695153b234a330 +0, 22, 22, 1, 21600, 6c298b196e16c64f7c2f407ba1242937 +0, 23, 23, 1, 21600, bf54474112ed5592c4d890e3313881a0 +0, 24, 24, 1, 21600, 945d49abedb0606b6a009c8b5d8face3 +0, 25, 25, 1, 21600, dd6ebef7b6f24619910de811918d3437 +0, 26, 26, 1, 21600, 7952346fc0f1eff3914e0d7646b3cf28 +0, 27, 27, 1, 21600, 26bd0d6b21e8a2df17af8d1446fba745 +0, 28, 28, 1, 21600, b0d91600416716d81c1f73ac141a0b62 +0, 29, 29, 1, 21600, 08f16698beb9cc15f7115961bd69e995 -- 1.7.10.4