[FFmpeg-devel] [PATCH] do not let claculated dts jump back

[Michael Niedermayer]

On Sat, Feb 22, 2014 at 05:22:48PM +0000, Rainer Hochecker wrote:
> Michael Niedermayer <michaelni <at> gmx.at> writes:
> 
> > > 
> > > 
> > > Yes, reverting 2ba68dd044ca8fc591139c05563840f546a9c0c0 cures the issue.
> > > 
> > > I don't see how calculation of dts from pts should work reliable with the
> > > value of num_reorder_frames. This value indicates the maximum number of
> > > frames in the reorder buffer (in this case 4). If we have a missing dts
> > > right after an idr frame, dts is set to a already passed pts.
> > > 
> > > I am wondering if it's better to leave dts at AV_NOPTS_VALUE instead of
> > > trying to calculate it. XBMC's player would be happy with this approach.
> > 
> > dts are needed for remuxing to containers which need dts, like
> > mpeg-ts/ps, so setting them nowhere isnt a good solution
> > 
> 
> I investigated you idea of changing delay. For the sample I provided
> has_b_frames is 4. Settings it to 2 gives me good results. When logging out
> slice types I see 2 b frames in a row so correcting this value may work.
> 
> Here is a new patch:
> 
> From 14dbb0492cb00b527ba2fe0e6c47097dad84d440 Mon Sep 17 00:00:00 2001
> From: Rainer Hochecker <fernetmenta at online.de>
> Date: Sat, 22 Feb 2014 18:21:30 +0100
> Subject: [PATCH] h264: measure num_b_frames
> 
> ---
>  libavcodec/h264.h        |  3 +++
>  libavcodec/h264_parser.c | 15 +++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/libavcodec/h264.h b/libavcodec/h264.h
> index 552a5af..ab427bd 100644
> --- a/libavcodec/h264.h
> +++ b/libavcodec/h264.h
> @@ -701,6 +701,9 @@ typedef struct H264Context {
>      AVBufferPool *mb_type_pool;
>      AVBufferPool *motion_val_pool;
>      AVBufferPool *ref_index_pool;
> +
> +    int b_frame_counter;         // counter consecutive b-frames
> +    int counted_reorder_frames;  // measured value for num_reordered_frames
>  } H264Context;
>  
>  extern const uint8_t ff_h264_chroma_qp[7][QP_MAX_NUM + 1]; ///< One chroma
> qp table for each possible bit depth (8-14).
> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
> index 4432871..7991a3d 100644
> --- a/libavcodec/h264_parser.c
> +++ b/libavcodec/h264_parser.c
> @@ -506,6 +506,21 @@ static int h264_parse(AVCodecParserContext *s,
>  
>      *poutbuf      = buf;
>      *poutbuf_size = buf_size;
> +
> +    // measure number of reordered frames
> +    // use this value instream of num_reordered_frames which is the maximum
> value
> +    if (buf_size) {
> +    	if (s->pict_type == AV_PICTURE_TYPE_B)
> +    		h->b_frame_counter++;
> +    	else {
> +    		if (h->b_frame_counter > h->counted_reorder_frames) {
> +    		    h->counted_reorder_frames = h->b_frame_counter;
> +    		}
> +    		h->b_frame_counter = 0;
> +    	}
> +    	if (0 < h->counted_reorder_frames < avctx->has_b_frames)
> +    		avctx->has_b_frames = h->counted_reorder_frames;

this looks wrong
you can have thousand b frame and the reorder buffer size still
can be 1

what problem remains with 3c096751ffe85861cd88ee822b86f10155b7b639

also the patch didnt appy, though that doesnt matter much in this
case

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140223/4a24da2a/attachment.asc>