[FFmpeg-devel] [PATCH] RoQ video encoder

[Michael Niedermayer]

Hi

On Thu, May 31, 2007 at 10:26:09PM -0400, Eric Lasota wrote:
[...]
> It looks like some incorrect changes were made to the decoder since I 
> last looked at it:
> roqvideodec.c line 94:
> case RoQ_ID_MOT:
>     ff_apply_motion_8x8(ri, xp, yp, 0, 0);
> 
> roqvideodec.c line 124:
> case RoQ_ID_MOT:
>     ff_apply_motion_4x4(ri, x, y, 0, 0);
> 
> ... Both of these are wrong.  MOT (or 0x0000 in the Q3 decoder) uses the 
> image section from two frames ago, not the previous frame.  More 
> specifically, MOT performs no action, leaving the image data that was 
> rendered in the buffer two frames ago.  This is the behavior described 
> by Ferguson's document as well.

hmm to me it seems the current behavior is teh same as what our initial
decoder did, so it at least is IMHO not a new bug

a patch to fix this is welcome (a sample which shows some artifacts with
the current decoder but works correctly with the ID decoder would also
be welcome if you have such a sample)


[...]
> Since the FFMPEG decoder already does double-buffering, a correct 
> implementation would be to only perform those motion applies on the 
> first frame, or just do what the official encoder does and copy the 
> first frame result to the backbuffer.

you dont seem to understand how the buffer handling in libavcodec works
there is no gurantee that the frame you get from get_buffer() will be
the same as the last you released with release_buffer()
you can use reget_buffer() to achive that ...


> 
> 
> Also, a minor untested patch to the RoQ encoder: An updated 
> SortPossibilityData that removes less-than-optimal reductions.  Might 
> produce a small increase in quality, might just cause blocking 
> artifacts, I haven't tested it extensively, it doesn't appear to cause 
> any quality degradation on basic tests.  It's formatted for libsb3, not 
> the FFMPEG port, so it needs to be cleaned up and ported to the new 
> struct names, but here you go:
> http://icculus.org/~riot/sortpossibilitydata.txt

could you explain how the roq encoder decides on the choice it makes
for encoding each block, ive not reviewed the highlevel design of it
carefully yet (i of course will before any patch with it will get
accepted) i was just confused a little by all the sorting ...

a quick highlevel overview would be appreciated


[...]
> I haven't been tracking the progress too well (just realized that Mike 
> Melanson decided to encourage reviving this effort after I benched it 2 
> years ago), but an issue that was brought up last time was that the 
> results are implementation-specific to qsort, to an extent that I can't 
> imagine ever having an impact on visual quality, but will produce 
> different binary output depending on how qsort is implemented if a equal 
> bit-error trades are encountered and ordered differently.  It might be 
> slightly faster to implement a qsort that uses a fixed size as well, 
> since all qsorts are performed on pointer lists.

you can make qsort behave binary identical by simply never returning a
0 from the comparission function

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070601/07cd66cf/attachment.pgp>