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

[Aurelien Jacobs]

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...

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: convergence_doc.diff
Type: text/x-diff
Size: 999 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100621/2dda5452/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: convergence_default.diff
Type: text/x-diff
Size: 476 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100621/2dda5452/attachment-0001.diff>