[FFmpeg-devel] [PATCH] mpeg2 non linear quantizer full support

[Michael Niedermayer]

On Tue, Mar 18, 2008 at 03:55:32PM +0100, Baptiste Coudurier wrote:
> Hi,
> 
> Michael Niedermayer wrote:
> > On Wed, Mar 12, 2008 at 07:38:42PM +0100, Baptiste Coudurier wrote:
> >> Hi,
> >>
> >> $subject.
> >>
> >> Diff for some files is to show possible simplifications of existing
> >> code. I'll of course split the commits. Only mpeg-2 unquantize functions
> >> are affected.
> >>
> >> Regression tests pass.
> > [...]
> > 
> > 
> >> Index: libavcodec/mpegvideo.c
> >> ===================================================================
> >> --- libavcodec/mpegvideo.c	(revision 12407)
> >> +++ libavcodec/mpegvideo.c	(working copy)
> >> @@ -33,6 +33,7 @@
> >>  #include "mpegvideo_common.h"
> >>  #include "mjpegenc.h"
> >>  #include "msmpeg4.h"
> >> +#include "mpeg12.h"
> >>  #include "faandct.h"
> >>  #include <limits.h>
> >>  
> >> @@ -2149,6 +2150,8 @@
> >>          block[0] = block[0] * s->y_dc_scale;
> >>      else
> >>          block[0] = block[0] * s->c_dc_scale;
> >> +
> >> +    qscale = s->qscale_table[qscale];
> > 
> > Doing the remaping in the dequant function means that its done 6 times (
> > 4 luma and 2 chroma) for 420, it might be more efficient to do it at some
> > oher place or maybe even keep qscale some linear quantizer scale instead
> > of a possibly non linear qp.
> > That is have 2 variables, qp which might be non linear and qscale which is
> > always the actual scale with which things get multiplied. Although iam not
> > sure if this really is better, maybe there are some problems iam missing ...
> > 
> > 
> 
> Ok, I added a qp field, which is updated in ff_set_qscale.
> Regression tests still pass.
[...]
> Index: libavcodec/mpegvideo_enc.c
> ===================================================================
> --- libavcodec/mpegvideo_enc.c	(revision 12488)
> +++ libavcodec/mpegvideo_enc.c	(working copy)
> @@ -34,6 +34,7 @@
>  #include "mjpegenc.h"
>  #include "msmpeg4.h"
>  #include "h263.h"
> +#include "mpeg12.h"
>  #include "faandct.h"
>  #include <limits.h>
>  
> @@ -75,13 +76,14 @@
>  static uint8_t default_mv_penalty[MAX_FCODE+1][MAX_MV*2+1];
>  static uint8_t default_fcode_tab[MAX_MV*2+1];
>  
> -void ff_convert_matrix(DSPContext *dsp, int (*qmat)[64], uint16_t (*qmat16)[2][64],
> -                           const uint16_t *quant_matrix, int bias, int qmin, int qmax, int intra)
> +void ff_convert_matrix(DSPContext *dsp, int (*qmat)[64], uint16_t (*qmat16)[2][64], const uint8_t *qscale_table,
> +                       const uint16_t *quant_matrix, int bias, int qmin, int qmax, int intra, uint64_t numerator)
>  {

s/qscale_table/qp2qscale_table/
(here and the other table names as well)

The idea behind the naming is that
QP     == (abstract) Quantization Parameter
qscale == linear (de)Quantization SCALE factor

This might need some seperate/cosmetic renaming, not sure ...


>      int qscale;
>      int shift=0;
>  
>      for(qscale=qmin; qscale<=qmax; qscale++){
> +        int qp = qscale_table ? qscale_table[qscale] : qscale;

exchange qp and qscale please


[...]
> @@ -1868,10 +1868,10 @@
>              /* add dct residue */
>              if(s->encoding || !(   s->h263_msmpeg4 || s->codec_id==CODEC_ID_MPEG1VIDEO || s->codec_id==CODEC_ID_MPEG2VIDEO
>                                  || (s->codec_id==CODEC_ID_MPEG4 && !s->mpeg_quant))){
> -                add_dequant_dct(s, block[0], 0, dest_y                          , dct_linesize, s->qscale);
> -                add_dequant_dct(s, block[1], 1, dest_y              + block_size, dct_linesize, s->qscale);
> -                add_dequant_dct(s, block[2], 2, dest_y + dct_offset             , dct_linesize, s->qscale);
> -                add_dequant_dct(s, block[3], 3, dest_y + dct_offset + block_size, dct_linesize, s->qscale);
> +                add_dequant_dct(s, block[0], 0, dest_y                          , dct_linesize, s->qp);
> +                add_dequant_dct(s, block[1], 1, dest_y              + block_size, dct_linesize, s->qp);
> +                add_dequant_dct(s, block[2], 2, dest_y + dct_offset             , dct_linesize, s->qp);
> +                add_dequant_dct(s, block[3], 3, dest_y + dct_offset + block_size, dct_linesize, s->qp);
>  
>                  if(!ENABLE_GRAY || !(s->flags&CODEC_FLAG_GRAY)){
>                      if (s->chroma_y_shift){
> @@ -1920,10 +1920,10 @@
>          } else {
>              /* dct only in intra block */
>              if(s->encoding || !(s->codec_id==CODEC_ID_MPEG1VIDEO || s->codec_id==CODEC_ID_MPEG2VIDEO)){
> -                put_dct(s, block[0], 0, dest_y                          , dct_linesize, s->qscale);
> -                put_dct(s, block[1], 1, dest_y              + block_size, dct_linesize, s->qscale);
> -                put_dct(s, block[2], 2, dest_y + dct_offset             , dct_linesize, s->qscale);
> -                put_dct(s, block[3], 3, dest_y + dct_offset + block_size, dct_linesize, s->qscale);
> +                put_dct(s, block[0], 0, dest_y                          , dct_linesize, s->qp);
> +                put_dct(s, block[1], 1, dest_y              + block_size, dct_linesize, s->qp);
> +                put_dct(s, block[2], 2, dest_y + dct_offset             , dct_linesize, s->qp);
> +                put_dct(s, block[3], 3, dest_y + dct_offset + block_size, dct_linesize, s->qp);

This change is wrong considering my definition of qp/qscale.


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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/20080319/8392c8ac/attachment.pgp>