[FFmpeg-devel] [PATCH] H.264 timestamps in h264_parser - complete set
Michael Niedermayer
michaelni
Tue Feb 24 20:58:49 CET 2009
On Tue, Feb 24, 2009 at 08:20:53PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
>> I ve just added h264 to tb_unreliable()
>>
> So, here the new patches:
>
> avcodec_timebase: change duration computation to use time_base instead of
> TB/2.
ok
> h264_timebase: correct time_base of H.264 and repeat_pict.
ok
>
> Further, I suppose, MPEG timebase should be adjusted as well, see
> mpeg_timebase (actually your patch), to handle repeat_pict correctly.
ok
>
> I also noticed that ffplay computes frame delay using old formula with 1/2
> fps. So I suppose, this should be changed as well (including increasing
> AVFrame.repeat_pict analogously to AVPacket.repeat_pict for H.264 and
> MPEG). Correct? If so, I could make a patch for it as well.
yes i think so
>
> In general, I wonder if repeat_pict (both in AVFrame and AVPacket) should
> not be renamed to frame_duration on next major version bump and increased
> by 1 everywhere...
maybe
>
> Timestamp computation is now also adjusted to your proposed names:
>
> avcodec_timestamp: add timestamp computation, if values exported from
> codec.
> h264_timestamp: timestamp parameter export from H.264.
>
> One problem remains, though: FPS detection doesn't take field pictures into
> account. So if the format returns frames containing field pictures with 1/2
> frame duration (or better to say, with DTS distance of 1/2 frame duration),
> then it will detect 2*fps instead of correct fps.
yes
>
> I'd propose adding a flag field_picture_flag to AVPacket (and pass it from
> the parser) to notify the user about field pictures, so it can correctly
> compute frame rate. What do you think? Or do you have a better idea?
i dont know, it doesnt seem to be such a big issue to me to have 2*fps ...
>
> > just added AVFMT_VARIABLE_FPS
>
> Shouldn't then mpegts have variable FPS as well? See mpegts_varframe patch.
no, mpeg-ps/ts do not support arbitrary variable fps also the flag is for
muxers
[...]
> @@ -3150,6 +3147,47 @@
> * subtitles are correctly displayed after seeking.
> */
> int64_t convergence_duration;
> +
> + // Timestamp generation support:
> + /**
> + * Synchronization point for start of timestamp generation.
> + *
> + * Set to >0 for sync point, 0 for no sync point and <0 for undefined
> + * (default).
> + *
> + * For example, this corresponds to presence of H.264 buffering period
> + * SEI message.
> + */
> + int dts_sync_point;
unneeded, the other 2 can just be set to AV_NOPTS_VALUE when unavailable
(they need to be 64bit then though)
> +
> + /**
> + * Offset of the current timestamp against last timestamp sync point in
> + * units of AVCodecContext.time_base.
> + *
> + * Set to INT_MIN when dts_sync_point unused. Otherwise, it must
> + * contain a valid timestamp offset.
> + *
> + * Note that the timestamp of sync point has usually a nonzero
> + * dts_ref_dts_delta, which refers to the previous sync point. Offset of
> + * the next frame after timestamp sync point will be usually 1.
> + *
> + * For example, this corresponds to H.264 cpb_removal_delay.
> + */
> + int dts_ref_dts_delta;
> +
> + /**
> + * Presentation delay of current frame in units of AVCodecContext.time_base.
> + *
> + * Set to INT_MIN when dts_sync_point unused. Otherwise, it must
> + * contain valid non-negative timestamp delta (presentation time of a frame
> + * must not lie in the past).
> + *
> + * This delay represents the difference between decoding and presentation
> + * time of the frame.
> + *
> + * For example, this corresponds to H.264 dpb_output_delay.
> + */
> + int pts_dts_delta;
> } AVCodecParserContext;
>
> typedef struct AVCodecParser {
> Index: libavformat/avformat.h
> ===================================================================
> --- libavformat/avformat.h (revision 17520)
> +++ libavformat/avformat.h (working copy)
> @@ -22,8 +22,8 @@
> #define AVFORMAT_AVFORMAT_H
>
> #define LIBAVFORMAT_VERSION_MAJOR 52
> -#define LIBAVFORMAT_VERSION_MINOR 29
> -#define LIBAVFORMAT_VERSION_MICRO 2
> +#define LIBAVFORMAT_VERSION_MINOR 30
> +#define LIBAVFORMAT_VERSION_MICRO 0
>
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> LIBAVFORMAT_VERSION_MINOR, \
> @@ -490,6 +490,16 @@
> const uint8_t *cur_ptr;
> int cur_len;
> AVPacket cur_pkt;
> +
> + // Timestamp generation support:
> + /**
> + * Timestamp corresponding to the last dts sync point.
> + *
> + * Initialized when AVCodecParserContext.dts_sync_point >= 0 and
> + * a DTS is received from the underlying container. Otherwise set to
> + * AV_NOPTS_VALUE by default.
> + */
> + int64_t reference_dts;
> } AVStream;
>
> #define AV_PROGRAM_RUNNING 1
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c (revision 17520)
> +++ libavformat/utils.c (working copy)
> @@ -837,6 +834,25 @@
> pkt->dts += offset;
> }
>
> + if (pc && pc->dts_sync_point >= 0) {
> + // we have synchronization info from the parser
> + int64_t den = st->codec->time_base.den * st->time_base.num;
you need a int64_t cast in there to prevent overflow
> + if (den > 0) {
> + int64_t num = st->codec->time_base.num * st->time_base.den;
same
> + if (pkt->dts != AV_NOPTS_VALUE) {
> + // got DTS from the stream, update reference timestamp
> + st->reference_dts = pkt->dts - pc->dts_ref_dts_delta * num / den;
av_rescale_q() should simplify this and similar code.
> + pkt->pts = pkt->dts + pc->pts_dts_delta * num / den;
> + } else if (st->reference_dts != AV_NOPTS_VALUE) {
> + // compute DTS based on reference timestamp
> + pkt->dts = st->reference_dts + pc->dts_ref_dts_delta * num / den;
> + pkt->pts = pkt->dts + pc->pts_dts_delta * num / den;
> + }
> + if (pc->dts_sync_point > 0)
> + st->reference_dts = pkt->dts; // new reference
> + }
a suggestion for simplification:
if (pkt->dts != AV_NOPTS_VALUE) {
st->prev_dts = pkt->dts;
} else if (st->prev_dts != AV_NOPTS_VALUE && dts_ref_dts_delta != AV_NOPTS_VALUE) {
st->prev_dts+= dts_ref_dts_delta * num / den;
pkt->dts = st->prev_dts;
}
if(pkt->pts == AV_NOPTS_VALUE && pkt->dts != AV_NOPTS_VALUE && pts_dts_delta != AV_NOPTS_VALUE);
pkt->pts = pkt->dts + pts_dts_delta * num / den;
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- 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/20090224/86272bc7/attachment.pgp>
More information about the ffmpeg-devel
mailing list