[FFmpeg-devel] [PATCH] DVCPRO HD - revised two-part patch

[Roman Shaposhnik]

On Fri, 2008-01-04 at 15:05 +0800, Dan Maas wrote:
> Thanks for your comments. After taking another look at the 
> find_macroblock functions, I've decided to withdraw that part of the 
> patch. The lookup tables are not as big as I thought (a few tens of KB) 
> and it was not hard to produce them for DV100. We can always switch to 
> functions later if it clearly improves performance.

  That's exactly the claim I would like to investigate. Please note,
that I'm not against or for the tables. I would like to have best
possible performance for the DV codec. That's all. So far your
statements confuse me. Whenever I'm confused or puzzled I experiment:

$ cat q.sh 
#!/bin/sh
prt() { grep "dezicycles" | sed -ne '$s/ .*$//p' ; }
while [ 1 = 1 ]; do
   new=`./ffmpeg.new -v 0 -debug 255 -y -t 90 -i /tmp/redhotridinghood.dv -f rawvideo -an /dev/null 2>&1 | prt` 
   old=`./ffmpeg -v 0 -debug 255 -y -t 90  -i /tmp/redhotridinghood.dv -f rawvideo -an /dev/null 2>&1 | prt old`
   echo "new=$new; old=$old; old is "`expr $new - $old`" dezicycles faster" 
done

$ ./q.sh
new=96482031; old=95858462; old is 623569 dezicycles faster
new=96649484; old=94936347; old is 1713137 dezicycles faster
new=96260970; old=94732728; old is 1528242 dezicycles faster
new=98264147; old=97990485; old is 273662 dezicycles faster

  Now, granted, the experiment is very quick and dirty and the slowdown
is not all that dramatic. But it is a slowdown nevertheless. So, please
answer the questions that I've asked in my original reply to you:

"Have you benchmarked it on the existing DV material or only
on DVCPRO HD? Also, what architectures did you run your tests on?
I'm a bit surprised when you say it could even be faster, since for the
original DV the lookup tables seemed to give us some advantage over the
computation of offsets."

> (I kept the 16-bit lookup tables for DV25/50 and added 32-bit tables for 
> DV100. If this is too ugly we could make the DV25/50 tables into 32-bits 
> or find another approach, let me know.)

  There's also a possibility of generating them on the fly inside
dvvideo_init. But again: whatever gives best performance.

> The new DVCPRO HD patch is in two parts. The first patch (attached) 
> makes all of the code changes necessary to support DVCPRO HD,

  Ok. This will take me some time to review. Stay tuned.

>  and the 
> second patch (URL below) adds the macroblock lookup tables.
> http://largefiles.maasdigital.com/largefiles/maas-dv100-2B-tables.diff.gz

  As far as the tables or no tables: I would like to see clear
performance results. If the results suggest that removing tables
(or generating them on the fly) is beneficial I would like to
see a *separate* patch be submitted to take care of DV25/50. The second
patch will be a purely DV100 patch, then. 

  Can you do this?

> You guys have better eyes for micro-optimizations than I do; if there 
> are more examples like Michael pointed out in the find_macroblock 
> functions, please let me know.

  I'll try to do my best, but Michael is an undisputed guru in this 
area ;-)

> One more thing I wanted to ask about - I have a compile-time #define for 
> changing a quality-speed tradeoff in the DV100 encoder. It would be 
> nicer to do this as a run-time switch, but I'm not sure how best to get 
> a quality-speed parameter down at the codec level. Any help would be 
> appreciated.

  Do you actually need this functionality? How much worse the
performance becomes? My point is simply that even for DV25/50 
we could introduce a similar thing, but I decided not to, since
the performance at the highest quality is quite competitive and
for people who are *really* in a hurry there's always lowres
option. ;-)

  Now, from my cursory reading of what that define does it 
looks like it almost can be viewed as a similar thing to
setting a custom Quantization Matrix for MPEG. Thus, perhaps the
best way to export it would be through the something similar
to -intra_matrix ?

Thanks,
Roman.