[Ffmpeg-devel] avcodec_find_best_pix_fmt1 matching problem

[Michael Niedermayer]

Hi

On Thu, Mar 01, 2007 at 04:25:35PM +0100, Panagiotis Issaris wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> Michael Niedermayer schreef:
> > Hi
> > 
> > On Thu, Mar 01, 2007 at 11:15:05AM +0100, Panagiotis Issaris wrote:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >> Hi,
> >>
> >> Panagiotis Issaris schreef:
> >>> Isn't there a problem on line 677 of this code?
> >>>
> >>> It appears to me that PIX_FMT_NB is to large (being 37), to allow the
> >>> bitshifting to work flawlessly.
> >>>
> >>>      666 static int avcodec_find_best_pix_fmt1(int pix_fmt_mask,
> >>>      667                                       int src_pix_fmt,
> >>>      668                                       int has_alpha,
> >>>      669                                       int loss_mask)
> >>>      670 {
> >>>      671     int dist, i, loss, min_dist, dst_pix_fmt;
> >>> [...]
> >>>      676     for(i = 0;i < PIX_FMT_NB; i++) {
> >>>      677         if (pix_fmt_mask & (1 << i)) {
> >>>      678             loss = avcodec_get_pix_fmt_loss(i, src_pix_fmt,
> >>> has_alpha) & loss_mask;
> >>> [...]
> >> The attached patch tries to fix the problem. It does change the
> >> signature of one of the public API functions though.
> >> avcodec_find_best_fix_fmt() which calls the above function wasn't used
> >> anywhere in the FFmpeg sourcetree, but ofcourse it might have been used
> >> by others.
> > 
> > changing from 32 to 64 is no solution, it will break too one day
> Yes, but it is broken right now, and the patch will fix it. It will be
> indeed break again when more than _27_ more pixel formats are added, but
> I do not see that as a reason to keep it broken as it is now.

problem is your change breaks ABI and after we add 27 more pixel formats
it would be broken again, if instead you would add an array of pix_fmts
the 2nd break could be avoided ...

[...]
-- 
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: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070301/9da01ad1/attachment.pgp>