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

[Nico Sabbi]

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