[Ffmpeg-devel] A question about 'dct_unquantize_h263_intra'

[Michael Niedermayer]

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>