[FFmpeg-devel] [PATCH] QCELP postfilter

[Michael Niedermayer]

On Thu, Apr 01, 2010 at 03:41:40PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Apr 1, 2010 at 8:37 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > i cannot decypher what you compared or if these are really psnr values
> > or stddev values
> > please list the unedited commands you use to encode, decode and compare
> > and the unedited output from tiny_psnr
> >
> > i also cannot comment on the rest of this mail as i have zero clue
> > what your psnr(a) and (b) values are
> 
> OK, let me try again.
> 
> My sample is schlangenmannliten.mov from mphq. Test command to decode
> using whatever code:
> yes | ./ffmpeg_g -i ~/Desktop/schlangenmannliten.mov -f wav -acodec
> pcm_s16le ~/Desktop/${outputfilename}.wav
> 
> I have 5 codebases:
> (1) current SVN, which is our FF float QCELP decoder, without
> postfilter (output called out-nopf.wav)
> (2) the patch in [1], which enables the reference decoder, but without
> postfilter, and one line to make it work for SVN:
> +    avctx->sample_fmt             = SAMPLE_FMT_S16; (output called
> out-ref-nopf.wav)
> (3) the same as above, but with one additional line in the init
> function to enable the postfilter:
> +    s->control.pf_flag = PF_ON; (output called out-ref-pf.wav)
> (4) current SVN with my postfilter patch (first patch in this thread)
> (output called out-with-ourpf.wav)
> (5) current SVN hacked to include the postfilter as in the reference
> code, qcelp/code/postfilt.c:postfilt() (output called
> out-with-refpf.wav)
> 
> And I also have several "intermediate" patches that swap gradually
> from "our code with reference postfilter" to "our code with our
> postfilter", each output will be called out-with-ourpf.wav also.
> 
> [1] http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2006-December/020223.html
> 
> First, our decoder (without postfilter) is not bitexact when compared
> to the reference decoder without postfilter:
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-{nopf,ref-nopf}.wav 2 0 44
> stddev:  281.97 PSNR: 47.33 bytes:  8820800/  8820800
> 
> Implementing the reference postfilter in our SVN decoder compared to
> reference decoder with postfilter enabled:
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-{ref-pf,with-refpf}.wav 2 0 44
> stddev:  310.92 PSNR: 46.48 bytes:  8820800/  8820800
> 
> So now, in the next few steps, I'll gradually switch from "our code
> with reference postfilter" to "our code with our postfilter" to show
> you how each step affects the PSNR/stddev. The command (comparing the
> curernt version against "reference decoder with postfilter" and "our
> decoder with reference postfilter") for each case is (in the example
> here, I'm using the starting code, i.e. "our decoder with reference
> postfilter"):
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-ref-pf.wav
> ~/Desktop/out-with-ourpf.wav 2 0 44
> stddev:  310.92 PSNR: 46.48 bytes:  8820800/  8820800 <- same as above
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-with-{ref,our}pf.wav 2 0 44
> stddev:    0.00 PSNR:999.99 bytes:  8820800/  8820800 <- well duh, I'm
> comparing it against itself
> 
> First thing that the postfilter does is IIR filtering (see [2]), very
> similar to AMR-NB decoder with slightly different coefficients. When I
> change that to use my own code (from my patch in the first msg in this
> thread), I get this:
> 
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-ref-pf.wav
> ~/Desktop/out-with-ourpf.wav 2 0 44
> stddev:  310.92 PSNR: 46.48 bytes:  8820800/  8820800
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-with-{ref,our}pf.wav 2 0 44
> stddev:    9.98 PSNR: 76.35 bytes:  8820800/  8820800
> 
> As you can see, stddev didn't change for the first command, but
> changed to +/- 10 for the second comparison. I'm not completely sure
> why and didn't look into this.
> 
> Second thing that the postfilter does is tilt correction. The ref code
> does tilt filtering with filtered samples, while our tilt code does it
> with unfiltered samples. The effect is quite dramatic, even though
> (when you think about it) it's purely an interpretation difference:
> 
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-ref-pf.wav
> ~/Desktop/out-with-ourpf.wav 2 0 44
> stddev:  334.97 PSNR: 45.83 bytes:  8820800/  8820800
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-with-{ref,our}pf.wav 2 0 44
> stddev:  145.32 PSNR: 53.08 bytes:  8820800/  8820800
> 
> Third thing that it does is adaptive gain correction (see [3]).
> However, instead of changing the gain correction per sample, it
> changes it per block of samples. Again, the effect of using our
> built-in function is dramatic, although it has no actual effect on
> audio quality (since all it does is adjust gain = amplitude):
> 
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-ref-pf.wav
> ~/Desktop/out-with-ourpf.wav 2 0 44
> stddev:  319.49 PSNR: 46.24 bytes:  8820800/  8820800
> bash-3.2$ tests/tiny_psnr ~/Desktop/out-with-{ref,our}pf.wav 2 0 44
> stddev:  194.41 PSNR: 50.55 bytes:  8820800/  8820800
> 
> And that's my patch in the thread start. Let me know if anything is
> unclear. IMO we can use the patch as-is, our goal isn't to replicate
> decoder behaviour at all cost, but rather to replicate high-quality
> audio output (again, IMO).

your mail nicely explains what you do different from the reference and
how much this changes the output from the reference (each change has a
very significant effect). What is missing is
1. which sounds better/closer to the original (double blind test)
2. which decoder output is closer to the encoer input by some
   comparission (like comparission in frequency domain throwing phase
   away) or plain psnr

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is not what we do, but why we do it that matters.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100401/09041e36/attachment.pgp>