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

[Ivan Schreter]

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: 3519 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090119/79a2c86b/attachment.bin>