[Ffmpeg-devel] [PATCH] pointer targets differ in signedness

[Michael Niedermayer]

Hi

On Tue, Sep 12, 2006 at 10:34:11PM +0100, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > Hi
> >
> > On Tue, Sep 12, 2006 at 08:50:01PM +0100, M?ns Rullg?rd wrote:
> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> 
> >> > Hi
> >> >
> >> > On Sun, Sep 10, 2006 at 06:36:16PM +0100, M?ns Rullg?rd wrote:
> >> >> This patch gets rid of many of the "pointer targets differ in
> >> >> signedness" warnings generated by gcc4.
> >> >> 
> >> >> Please, can those with knowledge of the files listed below check that
> >> >> I didn't do something stupid?  Needless to say, regression tests pass
> >> >> with these changes.
> >> > [...]
> >> >
> >> >>      int i;
> >> >>  
> >> >>      if(matrix){
> >> >> Index: libavcodec/mpegvideo.h
> >> >> ===================================================================
> >> >> --- libavcodec/mpegvideo.h	(revision 6221)
> >> >> +++ libavcodec/mpegvideo.h	(working copy)
> >> >> @@ -344,8 +344,8 @@
> >> >>      Picture *current_picture_ptr;  ///< pointer to the current picture
> >> >>      uint8_t *visualization_buffer[3]; //< temporary buffer vor MV visualization
> >> >>      int last_dc[3];                ///< last DC values for MPEG1
> >> >> -    int16_t *dc_val_base;
> >> >> -    int16_t *dc_val[3];            ///< used for mpeg4 DC prediction, all 3 arrays must be continuous
> >> >> +    uint16_t *dc_val_base;
> >> >> +    uint16_t *dc_val[3];           ///< used for mpeg4 DC prediction, all 3 arrays must be continuous
> >> >
> >> > iam not 100% sure if not maybe some msmpeg4 files had negative dc values
> >> > maybe it was a bug somewhere else, but as all coeffs are signed 16bit id
> >> > leave the dc stuff signed too just MHO
> >> 
> >> Those pointers are assigned to uint16_t * pointers, which are used for
> >> actual computations, so I assumed that unsigned was correct.  
> >
> > well, if you are brave change it to unsigned, if nothing breaks i wont
> > complain, OTOH if something does break, expect to drink a few 1000 liter
> > cola :)
> 
> Well, regression tests pass, for all that's worth.
> 
> >> How would the dc values end up negative anyway?
> >
> > predictor + signed value from bitstream
> 
> OK, but what kind of bizarre encoder would do that?  

well maybe one with a broken DCT or one which tries to change a few
coeffs by +-1 and then checks the error (or rate distortion) after the idct
due to cliping and rounding its possible that negative dc might be better in
some cases


> Wouldn't that
> make the AC values larger?  Nevertheless, if it is realistic to end up
> with signed DC values, I suppose we should be using signed numbers
> throughout.

as i said iam not sure about this, maybe its ok maybe not, maybe we even clip 
negative dc away everywhere where they could occur i dunno

also IIRC vp3 does dc prediction for non intra blocks which if so will be
negative sometimes, but then it seems that the patch didnt touch any code
related to vp3 dc ...


> 
> What about the rest of the patch?  There was a similar mismatch with
> the scaling matrixes, among other things.

the quantization matrixes should be unsigned of course
about the signed /unsigned char /uint8_t changes, ive not looked at them
closely
the startcode ->uint32_t changes are ok of course

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is