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

[Aurelien Jacobs]

On Mon, Jun 21, 2010 at 01:16:56PM +0200, Aurelien Jacobs wrote:
> On Mon, Jun 21, 2010 at 01:14:38PM +0200, Aurelien Jacobs wrote:
> > 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)
> 
> And here are the patches...

Ping ?

Aurel