Re: [FFmpeg-devel] [PATCH] h264dec: h264_select_output_frame() - fill correctly has_b_frames based on the level
Dne 9.11.2017 v 21:38 Jaroslav Kysela napsal(a): > The current code does not handle correctly the situation when > sps->num_reoder_frames > was set using the level. The incorrect has_b_frames results in the wrong DTS > guess and > malformed output (wrong 'Non-monotonous DTS' fixup). > > sps->num_reorder_frames is set in the function > ff_h264_decode_seq_parameter_set(), > see comment: 'if the maximum delay is not stored in the SPS, derive it based > on the > level'. > > Issue: #6810 > --- > libavcodec/h264_slice.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c > index 35dcabd611..04b10656e0 100644 > --- a/libavcodec/h264_slice.c > +++ b/libavcodec/h264_slice.c > @@ -1287,6 +1287,7 @@ static int h264_select_output_frame(H264Context *h) > h->mmco_reset = 0; > > if (sps->bitstream_restriction_flag || > +sps->ref_frame_count || > h->avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT) { > h->avctx->has_b_frames = FFMAX(h->avctx->has_b_frames, > sps->num_reorder_frames); > } > Hendrik Leppkes wrote: > This behavior has actually been intentional in the past to keep the > decode delay lower. The source of this check is: commit fb19e1443bc45e192545d3485ddb9c76e7d77855 Author: Michael Niedermayer Date: Sat Jul 19 16:16:00 2008 + Take the brain amputated spec literally if the user asks for it (-strict 1). That is, add 16 frames delay, cache trashing and av desync. fixes at least the following reference bitstreams: CABA3_Sony_C.jsv CACQP3_Sony_D.jsv CAMANL1_TOSHIBA_B.264 CANL3_Sony_C.jsv CVBS3_Sony_C.jsv CVMANL1_TOSHIBA_B.264 Originally committed as revision 14308 to svn://svn.ffmpeg.org/ffmpeg/trun But it does not take in account the level based num_reorder_frames (when ref_frame_count is available). I think that the original idea was to avoid to set has_b_frames unconditionally to 16. With the proposed change, has_b_frames are 4 for my problematic input stream. Again, the output stream is broken (timestamps) when the source stream has those parameters, so I think that the default behavior of ffmpeg should be changed. > If this is supposed to be changed (which needs to be discussed first), > the std_compliance value should probably be adjusted to NORMAL instead > of strict, instead of adding the additional condition here > (num_reorder_frames is only set from the profile if ref_frame_count is > > 0 in the first place). Perhaps, my original patch in issue #6810 is better then - remove all conditions and keep only: h->avctx->has_b_frames = FFMAX(h->avctx->has_b_frames, sps->num_reorder_frames); Please CC: me, I'm not subscribed to the list. Thanks, Jaroslav -- Jaroslav Kysela ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] h264dec: h264_select_output_frame() - fill correctly has_b_frames based on the level
On Thu, Nov 9, 2017 at 9:38 PM, Jaroslav Kysela wrote: > The current code does not handle correctly the situation when > sps->num_reoder_frames > was set using the level. The incorrect has_b_frames results in the wrong DTS > guess and > malformed output (wrong 'Non-monotonous DTS' fixup). > > sps->num_reorder_frames is set in the function > ff_h264_decode_seq_parameter_set(), > see comment: 'if the maximum delay is not stored in the SPS, derive it based > on the > level'. > > Issue: #6810 > --- > libavcodec/h264_slice.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c > index 35dcabd611..04b10656e0 100644 > --- a/libavcodec/h264_slice.c > +++ b/libavcodec/h264_slice.c > @@ -1287,6 +1287,7 @@ static int h264_select_output_frame(H264Context *h) > h->mmco_reset = 0; > > if (sps->bitstream_restriction_flag || > +sps->ref_frame_count || > h->avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT) { > h->avctx->has_b_frames = FFMAX(h->avctx->has_b_frames, > sps->num_reorder_frames); > } This behavior has actually been intentional in the past to keep the decode delay lower. If this is supposed to be changed (which needs to be discussed first), the std_compliance value should probably be adjusted to NORMAL instead of strict, instead of adding the additional condition here (num_reorder_frames is only set from the profile if ref_frame_count is > 0 in the first place). - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] h264dec: h264_select_output_frame() - fill correctly has_b_frames based on the level
The current code does not handle correctly the situation when sps->num_reoder_frames was set using the level. The incorrect has_b_frames results in the wrong DTS guess and malformed output (wrong 'Non-monotonous DTS' fixup). sps->num_reorder_frames is set in the function ff_h264_decode_seq_parameter_set(), see comment: 'if the maximum delay is not stored in the SPS, derive it based on the level'. Issue: #6810 --- libavcodec/h264_slice.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 35dcabd611..04b10656e0 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -1287,6 +1287,7 @@ static int h264_select_output_frame(H264Context *h) h->mmco_reset = 0; if (sps->bitstream_restriction_flag || +sps->ref_frame_count || h->avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT) { h->avctx->has_b_frames = FFMAX(h->avctx->has_b_frames, sps->num_reorder_frames); } -- 2.13.6 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel