[FFmpeg-devel] [PATCH] H.264 timestamps in h264_parser - complete set

[Michael Niedermayer]

On Thu, Feb 19, 2009 at 11:10:39AM +0100, Ivan Schreter wrote:
> Ivan Schreter wrote:
>> [...]
>>
>> BTW, I think we should then address also convergence_duration as well. I 
>> attached a second patch to add another field (and another minor version 
>> bump). Or should it go together in one minor version bump?
>>
> Attached is the patch which handles passing of convergence_duration from 
> codec to lavf correctly now. Please review.
>
> Regards,
>
> Ivan
>

> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h	(revision 17452)
> +++ libavcodec/avcodec.h	(working copy)
> @@ -30,7 +30,7 @@
>  #include "libavutil/avutil.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR 52
> -#define LIBAVCODEC_VERSION_MINOR 16
> +#define LIBAVCODEC_VERSION_MINOR 17
>  #define LIBAVCODEC_VERSION_MICRO  0
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> @@ -3033,6 +3033,14 @@
>       * will be used.
>       */

>      int key_frame;
> +    /*!

a empty line between these 2 would improve readability


> +     * Set by parser to number of frames until output synchronizes.
> +     * Initialized to 0.
> +     * Some codecs (e.g., H.264) may associate convergence duration with
> +     * a key frame. The output of the decoder after this many frames
> +     * after key frame will match output as if decoded from first frame.
> +     */
> +    int convergence_duration;
>  } AVCodecParserContext;
>  

i would prefer if convergence_duration in AVCodecParserContext where
defined the same way as in AVPacket, its a source of confusion otherwise


[...]
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 17452)
> +++ libavformat/utils.c	(working copy)
> @@ -899,15 +899,27 @@
>  //    av_log(NULL, AV_LOG_ERROR, "OUTdelayed:%d/%d pts:%"PRId64", dts:%"PRId64" cur_dts:%"PRId64"\n", presentation_delayed, delay, pkt->pts, pkt->dts, st->cur_dts);
> 
>      /* update flags */
> -    if(is_intra_only(st->codec))
> +    if(is_intra_only(st->codec)) {
>          pkt->flags |= PKT_FLAG_KEY;
> +        pkt->convergence_duration = 0;
> -    else if (pc) {
> +    } else if (pc) {
>          pkt->flags = 0;
>          /* keyframe computation */
> -        if (pc->key_frame == 1)
> +        if (pc->key_frame == 1) {
>              pkt->flags |= PKT_FLAG_KEY;
> +            if (pc->convergence_duration > 0) {
> +                int den = st->time_base.num * st->codec->time_base.den;
> +                if (den > 0)
> +                    pkt->convergence_duration = pc->convergence_duration *
> +                            st->time_base.den * st->codec->time_base.num / den;
> +                else
> +                    pkt->convergence_duration = 0;
> +            } else
> +                pkt->convergence_duration = 0;
> -        else if (pc->key_frame == -1 && pc->pict_type == FF_I_TYPE)
> +        } else if (pc->key_frame == -1 && pc->pict_type == FF_I_TYPE) {
>              pkt->flags |= PKT_FLAG_KEY;
> +            pkt->convergence_duration = 0;
> +        }

i dont see why convergence_duration should be tied to key_frame being
not -1


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20090219/7f82756e/attachment.pgp>