[FFmpeg-devel] [PATCH] h264 ticks and time base

[Michael Niedermayer]

On Sat, Jun 20, 2009 at 02:40:21AM -0700, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > On Fri, Jun 19, 2009 at 11:15:46PM -0700, Baptiste Coudurier wrote:
> >> Michael Niedermayer wrote:
> >>> On Wed, Jun 17, 2009 at 04:58:34PM -0700, Baptiste Coudurier wrote:
> >>>> Michael Niedermayer wrote:
> >>>>> On Sun, Jun 14, 2009 at 10:42:11PM -0700, Baptiste Coudurier wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> 1) Set avctx time_base in h264 parser.
> >>>>>> 2) Set avctx ticks_per_frame in h264 decoder.
> >>>>>>
> >>>>>> I don't get this code in decode_init in h264.c
> >>>>>>
> >>>>>>     reset_sei(h);
> >>>>>>     if(avctx->codec_id == CODEC_ID_H264){
> >>>>>>         if(avctx->ticks_per_frame == 1){
> >>>>>>             s->avctx->time_base.den *=2;
> >>>>>>         }
> >>>>>>         avctx->ticks_per_frame = 2;
> >>>>>>     }
> >>>>>>     return 0;
> >>>>>>
> >>>>>> avctx->ticks_per_frame is not set before calling the decoder
> >>>>>> in any case. Should that be removed ?
> >>>>> what do you mean by "that" ?
> >>>> the if(avctx->ticks_per_frame == 1)
> >>> i think it was needed because the codec might be ininted then closed and
> >>> then ininted again, but i may misremember
> >> Oh, hackish isn't it ?
> > 
> > do you have a better idea?
> 
> Removing it.
> 
> >> Nothing seems to set ticks_per_frame to 1 in h264 though.
> > 
> > no, 1 is the default
> > the first init of h264 sets it to 2 and doubles the time_base
> > the second must not double time_base again, the == 1 check
> > avoids it
> > if a demuxer meant to set the field rate instead of the frame rate
> > it can set ticks_per_frame to 2
> 
> Demuxer is not supposed to set avctx->time_base.
> Decoder has to override it in all case.
> 
> So my proposition is:
> Remove this code and always set it to 2.
> Do not set avctx->time_base in demuxer nor to r_frame_rate in
> av_find_stream_info.
> If timebase is not set, well it's not set and user knows it's unavailable.

iam not against this but it would need a few changes also iam not sure if
there are some cases that would end up needing hacks as well ...
One issue is that time_base is often not stored in the bitstream (some of
the h264 conformance streams fall in this category IIRC)
also our raw demuxers tend to set it and
time_base is used at various points (utils.c & ffmpeg.c at least) to estimate
the frame duration in absence of other hints, these would all become more
complex by a if(time_base not set try r_frame_rate)

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

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20090620/75ee6fae/attachment.pgp>