[FFmpeg-devel] [PATCH] H.264 timestamps in h264_parser - complete set
Michael Niedermayer
michaelni
Fri Feb 20 20:55:08 CET 2009
On Fri, Feb 20, 2009 at 01:24:30PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
>> On Wed, Feb 18, 2009 at 11:45:00PM +0100, Ivan Schreter wrote:
>>
>>> Michael Niedermayer wrote:
>>>
>>>> On Wed, Feb 18, 2009 at 12:33:57PM +0100, Ivan Schreter wrote:
>>>>
>>>>>> [...]
>>>>> Yes. I was thinking about creating additional function like
>>>>> ff_h264_decode_slice_header_0(), which would contain the first part of
>>>>> decode_slice_header(), which is relevant for parser. Do you have a
>>>>> better name suggestion?
>>>>>
>>>> not at the moment but maybe i come up with something later
>>>>
>>>>
>>> Unfortunately, it's not that simple, since the parsing code there is
>>> intermixed with other code related to decoding. I'm afraid to break
>>> things, not badly, but subtly. So I'd prefer not to factor it now, if you
>>> are OK with it. After the other OK-ed patches are committed, I'll post an
>>> updated version of parser patches.
>>>
>>
>> if you promise to factor it out in the end then ok but iam not entirely
>> happy about it
>>
>>
> I'll try to factor it out later.
>
> Here the complete patchset for timestamps again, adjusted to current state.
>
> #1: Export functions (already OK-ed).
> #2: Add rudimentary parser decoding relevant NALs and identifying picture
> type.
> #3: Add handling of key_frame flag to pass key frame info to lavf.
> #4: Add handling of convergence_duration to pass recovery frame count to
> lavf.
> #5: Add decoding of field flags.
> #6: Add decoding of H.264 timestamps and pass timestamps and duration to
> lavf (depends on my patch from today regarding passing duration to lavf).
>
> So please review.
>
> One question: H.264 standard specifies the timestamps hard-coded in 90kHz
> clock. So I hard-coded 90kHz in as well. Can it happen that some container
> uses non-90kHz clock? If yes, how do I find it out in lavc?
>
> Regards,
>
> Ivan
>
[...]
> + } else {
> + while(ptr[dst_length - 1] == 0 && dst_length > 0)
> + dst_length--;
> + bit_length= !dst_length ? 0 : (8*dst_length - ff_h264_decode_rbsp_trailing(h, ptr + dst_length - 1));
> + }
still seems useless
> +
> + init_get_bits(&h->s.gb, ptr, bit_length);
> + switch(h->nal_unit_type) {
> + case NAL_SPS:
> + ff_h264_decode_seq_parameter_set(h);
> + break;
> + case NAL_PPS:
> + ff_h264_decode_picture_parameter_set(h, h->s.gb.size_in_bits);
> + break;
> + case NAL_SEI:
> + ff_h264_decode_sei(h);
> + break;
> + case NAL_IDR_SLICE:
> + case NAL_SLICE:
> + get_ue_golomb(&h->s.gb); // skip first_mb_in_slice
> + slice_type = get_ue_golomb_31(&h->s.gb);
> + s->pict_type = golomb_to_pict_type[slice_type % 5];
missing validity checks
[...]
> Index: libavcodec/h264_parser.c
> ===================================================================
> --- libavcodec/h264_parser.c (revision 17392)
> +++ libavcodec/h264_parser.c (working copy)
> @@ -112,6 +112,7 @@
> {
> H264Context *h = s->priv_data;
> const uint8_t *buf_end = buf + buf_size;
> + int pps_id;
> unsigned int slice_type;
> int state;
> const uint8_t *ptr;
> @@ -176,6 +180,33 @@
> } else {
> s->convergence_duration = AV_NOPTS_VALUE;
> }
> + pps_id= get_ue_golomb(&h->s.gb);
> + if(pps_id>=MAX_PPS_COUNT) {
this check is insufficient, let me repeat, these checks are critical for
security please make tripple certain that you do not forget checking a
array index taken from the stream
[...]
> + if (s->dts != AV_NOPTS_VALUE) {
> + /*
> + * Each H.264 buffering period starts own sequence of timestamps.
> + * These are then computed contiguously for following buffering
> + * periods using given formulas above. When decoding from the
> + * beginning, the timestamps actually match those of the container
> + * (e.g., MPEG-TS) almost perfectly (with only a small offset).
> + *
> + * However, after a seek, when we start decoding in the middle of the
> + * stream, we don't have a timestamp history. So H.264 would start
> + * generating different (offset) timestamps.
> + *
> + * We want to actually generate timestamps in sync with the
> + * container format. Therefore, if the container does provide a DTS
> + * for the frame, adjust Trn(nb) timestamp to match timing of the
> + * container. Timestamp deltas are then computed using above mentioned
> + * H.264 formula.
> + */
> + if (h->sei_buffering_period_present) {
> + cpb_removal_delay = 0;
> + h->ts_trn_nb_format = s->dts;
> + } else if (cpb_removal_delay >= 0 && h->ts_trn_nb_format < 0) {
> + h->ts_trn_nb_format = s->dts -
> + cpb_removal_delay * h->ts_tc.num * 90000 / h->ts_tc.den;
> + }
> + }
> +
> + ts_trn_nb = h->ts_trn_nb_format; // use TS of the format
> + if (ts_trn_nb < 0)
> + ts_trn_nb = h->ts_trn_nb; // fallback until format TS available
> +
> + if (h->sps.pic_struct_present_flag) {
> + switch (h->sei_pic_struct) {
> + case SEI_PIC_STRUCT_TOP_FIELD:
> + case SEI_PIC_STRUCT_BOTTOM_FIELD:
> + duration = 1;
> + break;
> + case SEI_PIC_STRUCT_FRAME:
> + case SEI_PIC_STRUCT_TOP_BOTTOM:
> + case SEI_PIC_STRUCT_BOTTOM_TOP:
> + duration = 2;
> + break;
> + case SEI_PIC_STRUCT_TOP_BOTTOM_TOP:
> + case SEI_PIC_STRUCT_BOTTOM_TOP_BOTTOM:
> + duration = 3;
> + break;
> + case SEI_PIC_STRUCT_FRAME_DOUBLING:
> + duration = 4;
> + break;
> + case SEI_PIC_STRUCT_FRAME_TRIPLING:
> + duration = 6;
> + break;
> + }
> + } else {
> + duration = h->s.picture_structure == PICT_FRAME ? 2 : 1;
> + }
> +
> + if (cpb_removal_delay >= 0) {
> + h->cur_dts = ts_trn_nb +
> + cpb_removal_delay * h->ts_tc.num * 90000 / h->ts_tc.den;
> + } else {
> + // no removal delay specified, use best guess (add prev frame duration)
> + h->cur_dts += h->cur_duration;
> + }
> + pts = h->cur_dts +
> + h->sei_dpb_output_delay * h->ts_tc.num * 90000 / h->ts_tc.den;
> + h->cur_duration = duration * h->ts_tc.num * 90000 / h->ts_tc.den;
please dont round timestamps in the parser arbitrarily to 90khz
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/20090220/5ecc6f2b/attachment.pgp>
More information about the ffmpeg-devel
mailing list