[Ffmpeg-devel] clip_uint8

Aurelien Jacobs aurel
Mon May 1 01:56:23 CEST 2006


On Mon, 1 May 2006 01:05:47 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> Hi
> 
> On Sun, Apr 30, 2006 at 11:52:43PM +0200, Aurelien Jacobs wrote:
> [...]
> > 
> > > IMHO the behaviour should be documented and not changed
> > 
> > Hum... The behavior was somewhat unexpected when the result of
> > clip_uint8 is assigned to anything else than uint8_t. And after
> > this patch, I saw no behavior changes, no regression...
> > So I guess clip_uint8 was only used assigned to uint8_t. Which means
> > the (implicit) cast to uint8_t was done at some point anyway.
> > 
> > > or the cast to uint8_t could be done only on the overflow
> > > side of the if()
> > 
> > Do you mean that the following patch would be better ?
> 
> maybe ...
> either way, if you change the behaviour of a function you must
> check every case where its used, the case in mpegvideo.c for example
> contains a unneeded & 0xFF after either change

Right. I should have done this before.

> the function definitly behaved as expected, now it does something else
> iam not sure what we gain with that change? its not a bugfix, behaviour
> which is no longer what it was is still not documented

I'm currently writting a codec and tried to use clip_uint8 which seemed
to be what I want according to it's name. Unfortunatly it didn't behaved
as I expected. It would have needed a cast for each call or a &0xFF just
as in mpegvideo.c.

Anyway, now that I've checked it carefully, clip_uint8 is used only
with uint8_t vars, or in combination with &0xFF, so it's new behavior
seems to be the one intended in all it's current use.

So I now intend to apply the 2 attached patches.

Aurel
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: clip_doc.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20060501/7ac8213c/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mpegvideo_clip.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20060501/7ac8213c/attachment.asc>



More information about the ffmpeg-devel mailing list