[FFmpeg-devel] [PATCH 10/10] Document how av_cmp_q() deal when one of the values to be compared is 0/0.
Michael Niedermayer
michaelni
Mon Oct 4 01:54:21 CEST 2010
On Mon, Oct 04, 2010 at 12:14:34AM +0200, Stefano Sabatini wrote:
> On date Sunday 2010-10-03 23:37:19 +0200, Michael Niedermayer encoded:
> > On Sun, Oct 03, 2010 at 11:07:22PM +0200, Stefano Sabatini wrote:
> > > On date Saturday 2010-10-02 21:56:49 +0200, Michael Niedermayer encoded:
> > > > On Fri, Oct 01, 2010 at 09:16:55AM +0200, Tomas H?rdin wrote:
> > > > > On Thu, 2010-09-30 at 23:58 +0200, Michael Niedermayer wrote:
> > > > > > On Thu, Sep 30, 2010 at 09:45:41PM +0200, Stefano Sabatini wrote:
> > > > > > > ---
> > > > > > > libavutil/rational.h | 4 ++++
> > > > > > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > > >
> > > > > > > diff --git a/libavutil/rational.h b/libavutil/rational.h
> > > > > > > index 2dd0c2c..bd2e711 100644
> > > > > > > --- a/libavutil/rational.h
> > > > > > > +++ b/libavutil/rational.h
> > > > > > > @@ -41,6 +41,10 @@ typedef struct AVRational{
> > > > > > >
> > > > > > > /**
> > > > > > > * Compare two rationals.
> > > > > > > + * If one of the compared values is 0/0, the result of the comparison
> > > > > > > + * is always 0, so you should do an explicit check for that case
> > > > > > > + * rather than use this function.
> > > > > >
> > > > > > is there any return value that would in any case avoid an additional check?
> > > > >
> > > > > Perhaps AVERROR_INVALIDDATA?
> > > >
> > > > I think that if we do detect NAN then the most sensible value to return would
> > > > be INT_MIN because INT_MIN == -INT_MIN thus in some sense it is the best
> > > > non zero element that has both + and -
> > >
> > > Check attached patch. I have a slightly preference for the first patch
> > > I sent, as I like the idea of having a special value which results
> > > equal to all other rationals (0/0), also I fear that people may do
> > > something of the kind:
> > > if (av_cmp_q(...) < 0)
> > >
> > > in which case returning -INT_MIN with 0/0 will fail. On the other hand
> > > I have no strong objection, so if Michael prefer this form I won't
> > > object.
> > >
> > > Regards.
> > > --
> > > FFmpeg = Freak Fabulous Mean Prodigious Eretic Gigant
> >
> > > rational.h | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > 3a4323eb3fa490c69b78a1c662132d36ef75d66e 0004-Add-a-special-case-for-av_cmp_q-when-one-of-the-valu.patch
> > > From 9de2a819d33b0a5f35315ac07a2df2b22c6e86fb Mon Sep 17 00:00:00 2001
> > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > Date: Thu, 30 Sep 2010 21:42:56 +0200
> > > Subject: [PATCH 04/13] Add a special case for av_cmp_q() when one of the values to be
> > > compared is 0/0.
> > >
> > > ---
> > > libavutil/rational.h | 5 ++++-
> > > 1 files changed, 4 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/libavutil/rational.h b/libavutil/rational.h
> > > index 21542a8..e9933f4 100644
> > > --- a/libavutil/rational.h
> > > +++ b/libavutil/rational.h
> > > @@ -43,9 +43,12 @@ typedef struct AVRational{
> > > * Compare two rationals.
> > > * @param a first rational
> > > * @param b second rational
> > > - * @return 0 if a==b, 1 if a>b and -1 if a<b
> > > + * @return 0 if a==b, 1 if a>b, -1 if a<b, and INT_MIN if one of the
> > > + * values is of the form 0/0
> > > */
> > > static inline int av_cmp_q(AVRational a, AVRational b){
> > > + if (a.num == 0 && a.den == 0 || b.num == 0 && b.den == 0)
> > > + return INT_MIN;
> > > const int64_t tmp= a.num * (int64_t)b.den - b.num * (int64_t)a.den;
> >
> > you mix declaration and statement
> > i dont care but gcc 2.95 will hate you and maybe some other obscure compilers
>
> Yes I noticed after I sent the patch (and header missing).
>
> Regards.
> --
> FFmpeg = Fantastic and Friendly Most Portable Elected Geisha
> rational.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> dee04fec5d8b225643dd2279455a2823be014ba3 0004-Add-a-special-case-for-av_cmp_q-when-one-of-the-valu.patch
> From 399aec1c0a506813e0735966c1eb593625a40fe1 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Thu, 30 Sep 2010 21:42:56 +0200
> Subject: [PATCH 04/13] Add a special case for av_cmp_q() when one of the values to be
> compared is 0/0.
>
> ---
> libavutil/rational.h | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/libavutil/rational.h b/libavutil/rational.h
> index 21542a8..c7332c0 100644
> --- a/libavutil/rational.h
> +++ b/libavutil/rational.h
> @@ -28,6 +28,7 @@
> #ifndef AVUTIL_RATIONAL_H
> #define AVUTIL_RATIONAL_H
>
> +#include <limits.h>
> #include <stdint.h>
> #include "attributes.h"
>
> @@ -43,10 +44,13 @@ typedef struct AVRational{
> * Compare two rationals.
> * @param a first rational
> * @param b second rational
> - * @return 0 if a==b, 1 if a>b and -1 if a<b
> + * @return 0 if a==b, 1 if a>b, -1 if a<b, and INT_MIN if one of the
> + * values is of the form 0/0
> */
> static inline int av_cmp_q(AVRational a, AVRational b){
> const int64_t tmp= a.num * (int64_t)b.den - b.num * (int64_t)a.den;
> + if (a.num == 0 && a.den == 0 || b.num == 0 && b.den == 0)
> + return INT_MIN;
>
> if(tmp) return ((tmp ^ a.den ^ b.den)>>63)|1;
> else return 0;
this doesnt work with infinites
i would suggest:
Index: rational.h
===================================================================
--- rational.h (revision 25329)
+++ rational.h (working copy)
@@ -49,7 +49,9 @@
const int64_t tmp= a.num * (int64_t)b.den - b.num * (int64_t)a.den;
if(tmp) return ((tmp ^ a.den ^ b.den)>>63)|1;
- else return 0;
+ else if(b.den && a.den) return 0;
+ else if(a.num && b.num) return (a.num>>31) - (b.num>>31);
+ else return INT_MIN;
}
/**
if you agree ill commit that and leave the dox to you
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101004/42159dd7/attachment.pgp>
More information about the ffmpeg-devel
mailing list