[FFmpeg-devel] [PATCH 1/2] Revert "avcodec/qtrle: Do not output duplicated frames on insufficient input"

[Marton Balint]

On Tue, 7 May 2019, Michael Niedermayer wrote:

> On Sun, May 05, 2019 at 08:51:08PM +0200, Marton Balint wrote:
>> This reverts commit a9dacdeea6168787a142209bd19fdd74aefc9dd6.
>>
>> I don't think it is a good idea to drop frames from CFR input just because they
>> are duplicated, that can cause issues for API users expecting CFR input. Also
>> it can cause issues at the end of file, if the last frame is a duplicated
>> frame.
>>
>> Fixes ticket #7880.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavcodec/qtrle.c        |  12 ++---
>>  tests/ref/fate/qtrle-8bit | 109 ++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 115 insertions(+), 6 deletions(-)
>
> This change would make the decoder quite a bit slower.

I guess that can be easily fixed by only copying the buffer if it really 
is a different frame.

> It also would make encoding the output harder.
> For example motion estimation would be run over unchanged frames even when
> no cfr is wanted.

The performance penalty is much more acceptable to me than the issue 
described in the ticket. Do you see a straightforward way to fix it 
other than reverting?

>
> Also if one for consistency wants every decoder to not drop duplicated things
> that will cause some major problems in other decoders.
> Iam thinking of MPEG2 here, where the duplication is at a field level
> perfectly progressive material would be turned into some mess with field
> repetition in that case. Again undoing that in a subsequent stage would be
> quite a bit harder and wastefull
>
> so my oppinion is that its better to produce duplicated things only when
> needed and not always and hardcoded in the decoder.

The problem is that you are creating VFR from CFR behind the scenes even 
for files which are not meant to be VFR, so people don't expect to get VFR 
from them. There can be use cases where VFR generation is really 
beneficial, but it should not be the default IMHO. VFR is always more 
problematic, yes we mostly handle it, but not without issues here and 
there.

Regards,
Marton