[FFmpeg-devel] [PATCH] Implement in lavc a flag which makes avcodec_open() to choose the best framerate

[Michael Niedermayer]

On Sun, Aug 31, 2008 at 01:19:31PM +0200, Stefano Sabatini wrote:
> On date Sunday 2008-08-31 00:11:03 +0200, Michael Niedermayer encoded:
> [...]
> > I think we should add a function that takes a list of AVRational and a AVRational
> > and finds the closest match for that. Its return value should indicate if the
> > match is exact or approximate
> > Such function might be usefull for other things as well ...
> 
> Yes it's the best solution I can see, it's even simpler
> application-level-wise, I would like to have thought that!
> 

> I also thought about to return the comparation code rather than an
> "exact" flag like in av_reduce(), I slightly prefer the attached
> solution but let me know.

what about returning the actually found value, the user could then run
av_cmp_q() over it himself when he needs.


> 
> As for the name of the flag to implement in ffmpeg, I'm aware that
> -choose_best_framerate may sound too long, suggestions are welcome.

Id say always pick the best framerate that the codec supports, and allow
the user with -force-fps to override it.


[...]
> Index: libavutil/rational.h
> ===================================================================
> --- libavutil/rational.h	(revision 15120)
> +++ libavutil/rational.h	(working copy)
> @@ -113,4 +113,13 @@
>   */
>  AVRational av_d2q(double d, int max) av_const;
>  
> +/**
> + * Finds in \p q_list the rational nearest to \p q.

Finds the nearest value in q_list to q.


> + * @param nearest_q the pointer to the rational where to copy the
> + * nearest rational to \p q found
> + * @param q_list NULL-terminated list of rationals
> + * @return 1 if the found rational is equivalent to \p q, 0 otherwise
> + */
> +int av_find_nearest_q(AVRational *nearest_q, AVRational q, AVRational* q_list);
> +
>  #endif /* AVUTIL_RATIONAL_H */
> Index: libavutil/rational.c
> ===================================================================
> --- libavutil/rational.c	(revision 15119)
> +++ libavutil/rational.c	(working copy)
> @@ -101,3 +101,60 @@
>  
>      return a;
>  }
> +
> +int av_find_nearest_q(AVRational *nearest_q, AVRational q, AVRational* q_list)
> +{
> +    AVRational *p= q_list;

useless temporary variable


> +    AVRational best_error= (AVRational){INT_MAX, 1};

> +    nearest_q->num = nearest_q->den = 0;

unneeded


> +    for(; p->den!=0; p++){

the != 0 is superflous


> +        AVRational error= av_sub_q(q, *p);
> +        if(error.num <0) error.num *= -1;
> +        if(av_cmp_q(error, best_error) < 0){
> +            best_error= error;

> +            nearest_q->num = p->num;
> +            nearest_q->den = p->den;

*nearest_q= *p


> +        }
> +    }
> +    return !av_cmp_q(*nearest_q, q);
> +}
> +

> +#if TEST
> +int main (void) {
> +
> +    AVRational list_q[] = {
> +        {24000, 1001},
> +        {   24,    1},
> +        {   25,    1},
> +        {30000, 1001},
> +        {   30,    1},
> +        {   50,    1},
> +        {60000, 1001},
> +        {   60,    1},
> +        // Xing's 15fps: (9)
> +        {   15,    1},
> +        // libmpeg3's "Unofficial economy rates": (10-13)
> +        {    5,    1},
> +        {   10,    1},
> +        {   12,    1},
> +        {   15,    1},
> +        {    0,    0},
> +    };
> +
> +    AVRational q = { 5, 1};
> +    AVRational nearest_q;
> +    int exact= av_find_nearest_q(&nearest_q, q, list_q);
> +#undef printf
> +    printf ("q=%d/%d(%f), nearest_q=%d/%d(%f), exact=%d\n",
> +            q.num, q.den, (double)q.num/q.den,
> +            nearest_q.num, nearest_q.den, (double)nearest_q.num/nearest_q.den,
> +            exact);
> +
> +    q.num= 42, q.den = 1;
> +    exact= av_find_nearest_q(&q, q, list_q);
> +    printf ("q=%d/%d(%f), nearest_q=%d/%d(%f), exact=%d\n",
> +            q.num, q.den, (double)q.num/q.den,
> +            q.num, q.den, (double)q.num/q.den,
> +            exact);
> +}
> +#endif /* TEST */

hmm, i think if you cant come up with a shorter self test then drop it.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20080831/148186c2/attachment.pgp>