[FFmpeg-devel] [PATCH] fix convergence_duration default value

[Aurelien Jacobs]

On Thu, Jun 17, 2010 at 11:35:50AM +0200, Michael Niedermayer wrote:
> On Thu, Jun 17, 2010 at 11:24:51AM +0200, Aurelien Jacobs wrote:
> > On Thu, Jun 17, 2010 at 02:11:25AM +0200, Michael Niedermayer wrote:
> > > On Tue, Jun 15, 2010 at 11:19:54PM +0200, Aurelien Jacobs wrote:
> > > > Hi,
> > > > 
> > > > av_init_packet() currently initialize pkt->convergence_duration to 0.
> > > > This seems wrong. According to the documentation of convergence_duration:
> > > >     * Is AV_NOPTS_VALUE if unknown.
> > > > Moreover, av_parser_init() is following the documentation and initialize
> > > > convergence_duration to AV_NOPTS_VALUE.
> > > > I propose to fix av_init_packet() so that it also initialize
> > > > convergence_duration to AV_NOPTS_VALUE.
> > > 
> > > if its set to AV_NOPTS_VALUE by default then every demuxer has to
> > > override it to 0. if its 0 by default only demuxers where its acrually
> > > not 0 (or could b non 0) have to override
> > 
> > I'm not sure that I understand this correctly.
> > In my understanding, in most cases, convergence_duration can only be set
> > to 0 for actual key_frame. It must be different than 0 for P and B
> > frames.
> > So all demuxers supporting non-key frames would have to set
> > convergence_duration if its default value is 0 (none of them does it
> > right now, leaving convergence_duration to an incorrect value).
> > On the other end, I don't really see the point of having
> > convergence_duration set to 0 for key frames, as we already have this
> > information carried by pkt.flags == AV_PKT_FLAG_KEY.
> > 
> > So a default convergence_duration set to 0 seems useless and needs to be
> > overriden in many demuxers.
> > On the other end, if setting default convergence_duration to
> > AV_NOPTS_VALUE, it would need to be overriden only in a very few cases
> > (H264 SEI recovery point, and so on...) and wouldn't loose any
> > information.
> > 
> > Did I missed something ?
> 
> The idea was that convergence_duration has no meaning when AV_PKT_FLAG_KEY is
> NOT set.
> That way seeking to keyframes also makes sense and convergence_duration
> can make sense for audio (mdct) codecs that set AV_PKT_FLAG_KEY

Hum... OK. Then the doxy for convergence_duration should make this extra
clear. (see attached patch)

And av_parser_init() should also initialize convergence_duration to 0
for the same reasons. (see second patch)

Aurel