[FFmpeg-devel] [PATCH] libavcodec/escape130: fixes and improvements

Carl Eugen Hoyos ceffmpeg at gmail.com
Sat Jul 21 16:20:54 EEST 2018


2018-07-20 12:31 GMT+02:00, Michael Chaban <arsunt at gmail.com>:
> 2018-07-19 21:28 GMT+03:00 Carl Eugen Hoyos <ceffmpeg at gmail.com>:
>> The first commit breaks fate, you have to (confirm the fix and)
>> change the reference values in the same patch.
>
> The first commit fixes the wrong decoding which was error in first
> place for FFmpeg Escape130 implementation.

I understand

> The decoded frame examples and RPL video file were presented
> in the first message.

Yes.

> Sorry, It's my first time in contributing to FFmpeg, so I don't
> understand what exactly I have to do to make things right.

FFmpeg contains a test suite called "fate", you can run it by
first downloading the fate samples:
$ make SAMPLES=fate-suite fate-rsync
And then run the tests:
$ make SAMPLES=fate-suite -j2 fate
(or make V=1 SAMPLES... to see what's happening)

Your commit must not break fate, so you first confirm that
the change is intended for the given sample, and then
change the actual output with
$ make GEN=1 SAMPLES=fate-suite fate-armovie-escape130

> How to confirm the fix and change the reference values?

I just meant that you do some basic verification that the change
is not unintended for our sample.

> 2018-07-19 21:28 GMT+03:00 Carl Eugen Hoyos <ceffmpeg at gmail.com>:
>> Is there an alternative to the second patch? Like setting the
>> range correctly?
>
> Actually AVCOL_RANGE_JPEG can made the colorspace visually correct,
> since T-REC-T.871 is primarily used by JPEG. But the original decoder
> applies additional color patten to solid colour 2x2 pixel blocks (my
> commit 3/3). If I use AV_PIX_FMT_YUV420P I cannot apply the same RGB
> pattern. In this case I'll have to use Luma pattern instead, which is
> little different. What is more inportant: original decoder algorythm
> or YUV420P pixel format?

My personal opinion is that YUV420P makes more sense but cinepak
changed its output to RGB for the same (or a similar) reason so my
opinion may not be relevant.
It may come down to the question if the difference is (theoretically)
visible or not: Off-by-one cannot be visible.

Carl Eugen


More information about the ffmpeg-devel mailing list