diff mbox series

[FFmpeg-devel] avcodec/h264dec: add option to ignore in band parameter set

Message ID tencent_56A9004EB8255CB305C85EB3E8F36D15C805@qq.com
State New
Headers show
Series [FFmpeg-devel] avcodec/h264dec: add option to ignore in band parameter set | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished
andriy/make_aarch64_jetson success Make finished
andriy/make_fate_aarch64_jetson success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Zhao Zhili Feb. 6, 2022, 2:58 p.m. UTC
It works in three mode:
0: don't ignore in-band ps
1: ignore in-band ps
-1: if corrupted data is detected, then ignore in-band ps afterwards

h264dec working hard to do error resilience, it doesn't drop a
whole packet when error is detected in a nalu. Then there is a
higher chance for fake sps/pps be found and used. This happened
in a mp4 file. h264dec failed to recovery after broken data due
to the fake pps, while other H.264 decoders have no such problem.
---
 libavcodec/h264dec.c | 17 ++++++++++++++++-
 libavcodec/h264dec.h |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Hendrik Leppkes Feb. 6, 2022, 3:31 p.m. UTC | #1
On Sun, Feb 6, 2022 at 3:58 PM Zhao Zhili <quinkblack@foxmail.com> wrote:
>
> It works in three mode:
> 0: don't ignore in-band ps
> 1: ignore in-band ps
> -1: if corrupted data is detected, then ignore in-band ps afterwards
>
> h264dec working hard to do error resilience, it doesn't drop a
> whole packet when error is detected in a nalu. Then there is a
> higher chance for fake sps/pps be found and used. This happened
> in a mp4 file. h264dec failed to recovery after broken data due
> to the fake pps, while other H.264 decoders have no such problem.

This seems extremely hacky. I think you need to elaborate on the
problem you are trying to fix, so that any approach to a solution can
be judged properly.

- Hendrik
Zhao Zhili Feb. 6, 2022, 5:56 p.m. UTC | #2
> On Feb 6, 2022, at 11:31 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> 
> On Sun, Feb 6, 2022 at 3:58 PM Zhao Zhili <quinkblack@foxmail.com> wrote:
>> 
>> It works in three mode:
>> 0: don't ignore in-band ps
>> 1: ignore in-band ps
>> -1: if corrupted data is detected, then ignore in-band ps afterwards
>> 
>> h264dec working hard to do error resilience, it doesn't drop a
>> whole packet when error is detected in a nalu. Then there is a
>> higher chance for fake sps/pps be found and used. This happened
>> in a mp4 file. h264dec failed to recovery after broken data due
>> to the fake pps, while other H.264 decoders have no such problem.
> 
> This seems extremely hacky. I think you need to elaborate on the
> problem you are trying to fix, so that any approach to a solution can
> be judged properly.

There are two problems here:

Firstly, after split a packet into multiple NALUs, if part of these NALUs
have broken data, drop the whole packet or not. For H.264 with avcc
bitstream, the split itself is questionable if part of these NALUs are
invalid. Keep going after broken H264_NAL_SLICE has no serious effect,
but updating SPS/PPS in this case can be dangerous.

Secondly, for some containers like mp4, updating SPS/PPS are supported by
spec but don’t get widely support among implementations. FFmpeg can
generate mp4 with SPS/PPS updating in-band, but demux and decoding only
works without seek. On the whole, it make sense to have an option do
ignore in-band parameter set for those container.

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index a47caa95e8..1ee2b68459 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -681,6 +681,10 @@  static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
             break;
         case H264_NAL_SPS: {
             GetBitContext tmp_gb = nal->gb;
+            if (h->ps.sps && h->ignore_in_band_ps == 1) {
+                av_log(h, AV_LOG_WARNING, "ignore in-band sps\n");
+                break;
+            }
             if (avctx->hwaccel && avctx->hwaccel->decode_params) {
                 ret = avctx->hwaccel->decode_params(avctx,
                                                     nal->type,
@@ -700,6 +704,10 @@  static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
             break;
         }
         case H264_NAL_PPS:
+            if (h->ps.pps && h->ignore_in_band_ps == 1) {
+                av_log(h, AV_LOG_WARNING, "ignore in-band pps\n");
+                break;
+            }
             if (avctx->hwaccel && avctx->hwaccel->decode_params) {
                 ret = avctx->hwaccel->decode_params(avctx,
                                                     nal->type,
@@ -726,6 +734,8 @@  static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
         }
 
         if (err < 0) {
+            if (h->ignore_in_band_ps == -1)
+                h->ignore_in_band_ps = 1;
             av_log(h->avctx, AV_LOG_ERROR, "decode_slice_header error\n");
         }
     }
@@ -1011,8 +1021,12 @@  static int h264_decode_frame(AVCodecContext *avctx, void *data,
     }
 
     buf_index = decode_nal_units(h, buf, buf_size);
-    if (buf_index < 0)
+    if (buf_index < 0) {
+        /* If there is corrupted data, then don't trust the in-band ps. */
+        if (h->ignore_in_band_ps == -1)
+            h->ignore_in_band_ps = 1;
         return AVERROR_INVALIDDATA;
+    }
 
     if (!h->cur_pic_ptr && h->nal_unit_type == H264_NAL_END_SEQUENCE) {
         av_assert0(buf_index <= buf_size);
@@ -1055,6 +1069,7 @@  static const AVOption h264_options[] = {
     { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VDX },
     { "enable_er", "Enable error resilience on damaged frames (unsafe)", OFFSET(enable_er), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD },
     { "x264_build", "Assume this x264 version if no x264 version found in any SEI", OFFSET(x264_build), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VD },
+    { "ignore_in_band_ps", "ignore in-band parameter set", OFFSET(ignore_in_band_ps), AV_OPT_TYPE_INT, {.i64 = 0}, -1, 1, VD },
     { NULL },
 };
 
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index 8168c8e97b..760540bc9d 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -448,6 +448,7 @@  typedef struct H264Context {
     int bit_depth_luma;         ///< luma bit depth from sps to detect changes
     int chroma_format_idc;      ///< chroma format from sps to detect changes
 
+    int ignore_in_band_ps;
     H264ParamSets ps;
 
     uint16_t *slice_table_base;