diff mbox

[FFmpeg-devel,RFC/PATCH] lavc/vp8: Read a frame to set pix_fmt

Message ID CAB0OVGozurCPerYfcVpbS+2My7UXwCUTR5K+XiPWaF-G33GwJA@mail.gmail.com
State New
Headers show

Commit Message

Carl Eugen Hoyos Oct. 11, 2016, 8:29 a.m. UTC 
Hi!

Sorry for the delay!

2016-08-28 14:25 GMT+02:00 Ronald S. Bultje <rsbultje@gmail.com>:

> On Sun, Aug 28, 2016 at 7:24 AM, Carl Eugen Hoyos <cehoyos@ag.or.at> 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
diff mbox

Patch

From 7e9ffdb9e4385bc8c33e063266d282dc9125a6e7 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
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