[FFmpeg-devel] [PATCH] AVCHD/H.264 parser: determination of frame type, question about timestamps

[Michael Niedermayer]

On Fri, Jan 23, 2009 at 02:40:57PM +0100, Nico Sabbi wrote:
> On Friday 23 January 2009 09:02:20 Ivan Schreter wrote:
> > Hi,
> >
> > Loren Merritt wrote:
> > > On Thu, 22 Jan 2009, Ivan Schreter wrote
> > >
> > >> Your patch is much more elegant, but I changed the following:
> > >>> +        case NAL_IDR_SLICE:
> > >>> +        case NAL_SLICE:
> > >>> +            get_ue_golomb(&h->s.gb);
> > >>> +            if(get_ue_golomb(&h->s.gb) % 5 == 2)
> > >>> +                s->pict_type= FF_I_TYPE;
> > >>> +            else
> > >>> +                s->pict_type= FF_P_TYPE;
> > >>> +            return;
> > >>> +
> > >>
> > >> to this:
> > >>
> > >> +        case NAL_IDR_SLICE:
> > >> +        case NAL_SLICE:
> > >> +            get_ue_golomb(&h->s.gb);
> > >> +            s->pict_type=
> > >> golomb_to_pict_type[get_ue_golomb(&h->s.gb) % 5]; +           
> > >> return;
> > >>
> > >> Reason: Your code didn't correctly set pict_type to FF_B_TYPE
> > >> for B-frames, so timing didn't work correctly. Since we already
> > >> have mapping array, I simply used it to map it instead of adding
> > >> another if statement.
> > >
> > > Certainly frame type should be fixed, but what does this have to
> > > do with timing? H.264 allows any frame types to be reordered, or
> > > not, as the encoder pleases.
> >
> > This part has nothing to do with timing, only with returning proper
> > frame type.
> >
> > There is also a change in mpegts (code from Baptiste):
> >
> >     --- libavformat/mpegts.c        (revision 16686)
> >     +++ libavformat/mpegts.c        (working copy)
> >     @@ -888,7 +888,7 @@
> >                      pes->pts = AV_NOPTS_VALUE;
> >                      pes->dts = AV_NOPTS_VALUE;
> >                      if ((flags & 0xc0) == 0x80) {
> >     -                    pes->pts = get_pts(r);
> >     +                    pes->dts = pes->pts = get_pts(r);
> >                          r += 5;
> >                      } else if ((flags & 0xc0) == 0xc0) {
> >                          pes->pts = get_pts(r);
> >
> > this makes sure that frames having only PTS will get the same DTS
> > (note: only for mpegts). Otherwise, libavformat gets confused and
> > computes wrong DTS timestamps, which in turn results in
> > dropping/duplicating frames in ffmpeg.c while recoding.
> >
> > There are further problems, which my patch doesn't address. Current
> > patch only allows decoding progressive AVCHD files from my
> > camcorder. Interlaced don't work yet, but without (at least some)
> > info I asked for in my previous mail I cannot really analyze the
> > problem.
> >
> > Regards,
> >
> > Ivan
> 
> 
> this patch is wrong: if the DTS is not set in the PES there's no 
> reason to assum that it's equal to the PTS. If it's a workround for a 
> bug in the parser the parser should be fixed instead

no, you too should read the spec again :)
--------
H.222.0 2.7.5
...
    A decoding_timestamp (DTS) shall appear in a PES packet header if and only if the following two conditions are met:
             a PTS is present in the PES packet header;
             the decoding time differs from the presentation time.
--------

Now if a pts is present in the PES packet header AND dts!=pts then by this
rule the pts AND dts MUST be coded. The fact that it is not in this branch
implicates that pts==dts


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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20090123/245f9456/attachment.pgp>