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
