h264: Try reading SPS with incorrect escaping

Message ID 1331593660-36546-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö March 12, 2012, 11:07 p.m.
From: Michael Niedermayer <michaelni@gmx.at>

This fixes playback of
http://streams.videolan.org/streams/mp4/Mr_MrsSmith-h264_aac.mp4,
which has the forbidden byte sequence 0x00 0x00 0x00 in
its SPS.
---
 libavcodec/h264.c    |    6 +++++-
 libavcodec/h264_ps.c |    5 ++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Ronald Bultje March 12, 2012, 11:19 p.m. | #1
Hi,

On Mon, Mar 12, 2012 at 4:07 PM, Martin Storsjö <martin@martin.st> wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
>
> This fixes playback of
> http://streams.videolan.org/streams/mp4/Mr_MrsSmith-h264_aac.mp4,
> which has the forbidden byte sequence 0x00 0x00 0x00 in
> its SPS.
> ---
>  libavcodec/h264.c    |    6 +++++-
>  libavcodec/h264_ps.c |    5 ++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 20aed27..25aa8ab 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -3925,7 +3925,11 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size){
>             break;
>         case NAL_SPS:
>             init_get_bits(&s->gb, ptr, bit_length);
> -            ff_h264_decode_seq_parameter_set(h);
> +            if (ff_h264_decode_seq_parameter_set(h) < 0 && h->is_avc && (nalsize != consumed) && nalsize) {
> +                av_log(h->s.avctx, AV_LOG_DEBUG, "SPS decoding failure, trying alternative mode\n");
> +                init_get_bits(&s->gb, &buf[buf_index + 1 - consumed], 8*nalsize);
> +                ff_h264_decode_seq_parameter_set(h);
> +            }
>
>             if (s->flags& CODEC_FLAG_LOW_DELAY ||
>                 (h->sps.bitstream_restriction_flag && !h->sps.num_reorder_frames))
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 287702c..32398ec 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -228,7 +228,6 @@ static inline int decode_vui_parameters(H264Context *h, SPS *sps){
>         get_ue_golomb(&s->gb); /*max_dec_frame_buffering*/
>
>         if (get_bits_left(&s->gb) < 0) {
> -            av_log(h->s.avctx, AV_LOG_ERROR, "Overread VUI by %d bits\n", -get_bits_left(&s->gb));
>             sps->num_reorder_frames=0;
>             sps->bitstream_restriction_flag= 0;
>         }
> @@ -238,6 +237,10 @@ static inline int decode_vui_parameters(H264Context *h, SPS *sps){
>             return -1;
>         }
>     }
> +    if (s->gb.size_in_bits < get_bits_count(&s->gb)) {

get_bits_left(&s->gb) < 0.

Otherwise OK, you can push with that changed.

Ronald
Janne Grunau March 12, 2012, 11:39 p.m. | #2
On 2012-03-13 01:07:40 +0200, Martin Storsjö wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> This fixes playback of
> http://streams.videolan.org/streams/mp4/Mr_MrsSmith-h264_aac.mp4,
> which has the forbidden byte sequence 0x00 0x00 0x00 in
> its SPS.
> ---
>  libavcodec/h264.c    |    6 +++++-
>  libavcodec/h264_ps.c |    5 ++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 20aed27..25aa8ab 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -3925,7 +3925,11 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size){
>              break;
>          case NAL_SPS:
>              init_get_bits(&s->gb, ptr, bit_length);
> -            ff_h264_decode_seq_parameter_set(h);
> +            if (ff_h264_decode_seq_parameter_set(h) < 0 && h->is_avc && (nalsize != consumed) && nalsize) {
> +                av_log(h->s.avctx, AV_LOG_DEBUG, "SPS decoding failure, trying alternative mode\n");
> +                init_get_bits(&s->gb, &buf[buf_index + 1 - consumed], 8*nalsize);

this has an off by one error, first byte of the NAL skipped but not
subtracted from the size.

> +                ff_h264_decode_seq_parameter_set(h);
> +            }
>  
>              if (s->flags& CODEC_FLAG_LOW_DELAY ||
>                  (h->sps.bitstream_restriction_flag && !h->sps.num_reorder_frames))
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 287702c..32398ec 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -228,7 +228,6 @@ static inline int decode_vui_parameters(H264Context *h, SPS *sps){
>          get_ue_golomb(&s->gb); /*max_dec_frame_buffering*/
>  
>          if (get_bits_left(&s->gb) < 0) {
> -            av_log(h->s.avctx, AV_LOG_ERROR, "Overread VUI by %d bits\n", -get_bits_left(&s->gb));
>              sps->num_reorder_frames=0;
>              sps->bitstream_restriction_flag= 0;
>          }
> @@ -238,6 +237,10 @@ static inline int decode_vui_parameters(H264Context *h, SPS *sps){
>              return -1;
>          }
>      }
> +    if (s->gb.size_in_bits < get_bits_count(&s->gb)) {
> +        av_log(h->s.avctx, AV_LOG_ERROR, "Overread VUI by %d bits\n", get_bits_count(&s->gb) - s->gb.size_in_bits);
> +        return -1;
> +    }
>  
>      return 0;
>  }

pushed my version

Janne

Patch

diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index 20aed27..25aa8ab 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -3925,7 +3925,11 @@  static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size){
             break;
         case NAL_SPS:
             init_get_bits(&s->gb, ptr, bit_length);
-            ff_h264_decode_seq_parameter_set(h);
+            if (ff_h264_decode_seq_parameter_set(h) < 0 && h->is_avc && (nalsize != consumed) && nalsize) {
+                av_log(h->s.avctx, AV_LOG_DEBUG, "SPS decoding failure, trying alternative mode\n");
+                init_get_bits(&s->gb, &buf[buf_index + 1 - consumed], 8*nalsize);
+                ff_h264_decode_seq_parameter_set(h);
+            }
 
             if (s->flags& CODEC_FLAG_LOW_DELAY ||
                 (h->sps.bitstream_restriction_flag && !h->sps.num_reorder_frames))
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 287702c..32398ec 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -228,7 +228,6 @@  static inline int decode_vui_parameters(H264Context *h, SPS *sps){
         get_ue_golomb(&s->gb); /*max_dec_frame_buffering*/
 
         if (get_bits_left(&s->gb) < 0) {
-            av_log(h->s.avctx, AV_LOG_ERROR, "Overread VUI by %d bits\n", -get_bits_left(&s->gb));
             sps->num_reorder_frames=0;
             sps->bitstream_restriction_flag= 0;
         }
@@ -238,6 +237,10 @@  static inline int decode_vui_parameters(H264Context *h, SPS *sps){
             return -1;
         }
     }
+    if (s->gb.size_in_bits < get_bits_count(&s->gb)) {
+        av_log(h->s.avctx, AV_LOG_ERROR, "Overread VUI by %d bits\n", get_bits_count(&s->gb) - s->gb.size_in_bits);
+        return -1;
+    }
 
     return 0;
 }