Hi
On Tue, Jan 02, 2007 at 10:22:26PM +0200, Siarhei Siamashka wrote:
> On Tuesday 02 January 2007 19:44, Michael Niedermayer wrote:
[...]
> > > 3. Processing extra elements after block[nCoeffs] is safe (up to but not
> > > including block[(nCoeffs + 7) / 8 * 8])?
> >
> > block[0 .. 63] is always safe
> > nCoeffs <= 64
>> So overwriting values in this buffer with garbage after nCoeffs is ok and they
> are not used later anyway? Or at least processing them in the same way as
> the previous elements is ok?
the elements after nCoeffs should be 0 before and after *unquantize*
>> > > +static inline void dct_unquantize_h263_helper_c(DCTELEM *block, int
> > > qmul, int qadd, int count) +{
> > > + int i, level;
> > > + for (i = 0; i < count; i++) {
> > > + level = block[i];
> > > + if (level) {
> > > + if (level < 0) {
> > > + level = level * qmul - qadd;
> > > + } else {
> > > + level = level * qmul + qadd;
> > > + }
> > > + block[i] = level;
> > > + }
> > > + }
> > > +}
> >
> > this looks like a duplicate of dct_unquantize_h263_inter_c() ?
>> Yes, it is a subset of code that is used in dct_unquantize_h263_inter_c().
> Anyway, the reason for have this part of code moved to a separate function is
> to easily do correctness and performance tests of assembly optimized code. It
> is used as a reference implementation and dct_unquantize_h263_helper_armv5te()
> macro (armv5te optimized implementation) is directly compared to it. Both
> these implementations are used from 'test-unquantize.c' test program:
>https://garage.maemo.org/svn/mplayer/trunk/libavcodec/tests/>> I generally don't like the idea of developing assembly code unless it is
> (almost) completely covered by regression tests. Also as the goal of assembly
> optimizations is performance, tests which measure performance are also very
> important. Now as I have this simple test program, I can do with the assembly
> code whatever I like and most of the bugs are immediately discovered. Also I
> can immediately see performance results of different variations of code and
> ask other people to run benchmarks on other machines.
>> My plans also include adding some tests for idct later (in fact I have some of
> the test code, but it needs to be cleaned up a bit). It also should verify
> correctness and performance (also simulating real memory usage patterns -
> accesses to 'DCTELEM *block' always hit cache, but 'uint8_t *dest' has a lot
> of cache misses). The last part of tests will include motion compensation
> code. After all this code is covered by tests, I will start optimizing it
> quite aggresively. The same framework will be also useful if somebody would
> like to optimize code for iwmmxt (xscale) or armv6 SIMD instructions.
>> So while this function is obviously a duplication of code, I don't feel like
> removing it unless some better solution exists which would allow to keep
> regression tests. What is the preferred method of testing assembly code in
> ffmpeg now?
* make test
* START/STOP_TIMER around stuff to test speed
* some code under #ifdef TEST in some files like for example libavutil/tree.c
anyway i dont care about code duplication if its under a #if 0 or #ifdef TEST
but if it is compiled (twice) then i do mind ...
[...]
>> > > +#define dct_unquantize_h263_helper_armv5te(__block, __qmul, __qadd,
> > > __count) \
> >
> > things starting with __ are reserved in C please dont use such names
>> Thanks, I'll rename these identifiers, that's not a problem.
>> By the way, what is 's->h263_aic' in this code? If it is set, there should be
> possible to have a faster shortcut version for 'qadd = 0' branch. What kind of
> codec uses it and how can I encode video to get some test sample?
"-flags aic" with "-vcodec h263p" or so should work
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20070102/0bb89002/attachment.pgp>