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

[Ivan Schreter]

Hi,

I suppose, my mail with the patch got lost in the traffic somehow. So I
resubmit the patch here (which is basically the changes from Michael &
Baptiste with one minor modification). Below the full text of my mail.

Any further comments?

Thanks & regards,

Ivan

---------------------------------------

Hi Michael & Baptiste,

Michael Niedermayer wrote:
> you are duplicating h264.c, dont do this
> and if you now argue you just need this little bit, no you need 50 times
> more for the timestamps
>   
Correct, and I won't argue with you :-).
> my current code for cleaning things up is below, feel free to use it
> for your future patches, i seem to be too lazy to work on it :(
>   
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.


Baptiste Coudurier wrote:
> Humm this might need checking, at least ts demuxer only sets pts
> currently when only pts is available. Attached patch fixes this.
>   
Yes, I can also confirm, your patch now fixes DTS/PTS problem for frames
missing DTS (but not for those missing both).

Complete patch with code from Michael and Baptiste with my modification
regarding B-frames is attached. With this patch, it's possible to
correctly parse *progressive* AVCHD video produced by recent AVCHD
camcorders (like from my Panasonic HDC-SD9).

Would any of ffmpeg maintainer care to apply the patch now?

However, there is still problem with timestamps in case of *interlaced*
video. Namely, currently every second half-frame comes without DTS/PTS.
Thus, compute_pkt_fields() tries to assign DTS/PTS to it, but it does it
incorrectly and also IMHO has no way to do it really correctly with
current information.

Michael Niedermayer wrote:
> read the specs: H264 & H222
>   
Unfortunately, H.264 spec which I found in internet doesn't mention much
detail about how interlaced frames are to be timed. H.222 is supposedly
superseded with H.221, but again, no information about interlacing there.

H.264 spec only says: "The two fields of an interlaced frame are
separated in capture time while the two fields of a progressive frame
share the same capture time". That means, I was essentially correct in
my assumption - the half-frames without DTS/PTS in interlaced AVCHD
which represent the second half of interlaced picture have to be offset
by 1/2 of 25fps frame duration or by 1 50fps frame duration from first
half-frame. I.e., it looks like for interlaced videos ffmpeg works with
wrong frame duration in compute_pkt_fields().

A workaround by simply not assigning PTS/DTS in compute_pkt_fields() at
all in utils.c like this:

@@ -811,9 +829,29 @@
          pkt->dts= pkt->pts= AV_NOPTS_VALUE;
      }

+    if (pkt->dts == pkt->pts && pkt->dts == AV_NOPTS_VALUE)
+        return;
+
      if (pkt->duration == 0) {
          compute_frame_duration(&num, &den, st, pc, pkt);
          if (den && num) {

actually fixes the problem. But I expect, this is not the correct
solution, since it will possibly cause some other errors elsewhere,
especially if PTS is missing on several frames in a row... So I feel we
need to store last PTS/DTS and in this special case and only if
interlaced and last PTS/DTS set, use last PTS/DTS + half frame duration
and reset last PTS/DTS afterwards.

So here some questions:

Is the frame duration a ffmpeg-internal thing, or is it also described
by a standard (if so, I didn't find it)?

Should the frame rate be actually 50 instead of 25 for 50i videos?

Is there a possibility to find out whether the movie is interlaced
inside of compute_pkt_fields()? I'd like to write a preliminary patch
which can handle timing correctly for AVCHD 50i videos, but didn't find
any easy way to get this information.

Further, with av_read_frame, we are actually reading a half-frame for
interlaced video, right? At least it seems to return 50 times per second
for AVCHD 50i. Then, of course, decoding will create 25 interlaced
pictures per second.

Regarding the seeking problem of AVCHD streams, I'll open a new thread
when the parsing works correctly. If you care, you can give me some
pointers as to where to look in h264 code. Seems like h264 codec doesn't
"forget" some old frames in buffers when seeking to random location,
which causes additional artefacts and essentially prevents seeking.
Also, it seems it doesn't restart correctly at an I-frame after seeking.

Thanks for answering my (possibly ignorant) questions.

Regards,

Ivan




-------------- next part --------------
A non-text attachment was scrubbed...
Name: h264_patch.diff
Type: text/x-patch
Size: 3521 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090122/1ecefd97/attachment.bin>