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

[Carl Eugen Hoyos]

Hi!

Sorry for the delay!

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

> On Sun, Aug 28, 2016 at 7:24 AM, Carl Eugen Hoyos <cehoyos at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavc-vp8-Read-a-frame-to-set-pix_fmt.patch
Type: text/x-patch
Size: 7005 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161011/6adec001/attachment.bin>