[FFmpeg-devel] fix GetBitContext index when SPS bitstream_restriction_flag=1 but can't be parsed, because index is out of bit-stream size. Need return index value continue H.264 parsing

Submitted by Ivan Shmakov on May 16, 2017, 12:13 p.m.

Details

Message ID 000901d2ce3d$d8db8e00$8a92aa00$@zingaya.com
State New
Headers show

Commit Message

Ivan Shmakov May 16, 2017, 12:13 p.m.
---
 libavcodec/h264_ps.c | 4 ++++
 1 file changed, 4 insertions(+)

 
@@ -221,8 +222,10 @@ static inline int decode_vui_parameters(GetBitContext
*gb, AVCodecContext *avctx
     sps->pic_struct_present_flag = get_bits1(gb);
     if (!get_bits_left(gb))
         return 0;
+
     sps->bitstream_restriction_flag = get_bits1(gb);
     if (sps->bitstream_restriction_flag) {
+        idx = gb->index;
         get_bits1(gb);     /* motion_vectors_over_pic_boundaries_flag */
         get_ue_golomb(gb); /* max_bytes_per_pic_denom */
         get_ue_golomb(gb); /* max_bits_per_mb_denom */
@@ -234,6 +237,7 @@ static inline int decode_vui_parameters(GetBitContext
*gb, AVCodecContext *avctx
         if (get_bits_left(gb) < 0) {
             sps->num_reorder_frames         = 0;
             sps->bitstream_restriction_flag = 0;
+            gb->index = idx;
         }
 
         if (sps->num_reorder_frames > 16U

Comments

Hendrik Leppkes May 16, 2017, 8:02 p.m.
On Tue, May 16, 2017 at 2:13 PM, Ivan Shmakov <ishmakov@zingaya.com> wrote:
> ---
>  libavcodec/h264_ps.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 7858361..09e2290 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -136,6 +136,7 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
>  {
>      int aspect_ratio_info_present_flag;
>      unsigned int aspect_ratio_idc;
> +    int idx;
>
>      aspect_ratio_info_present_flag = get_bits1(gb);
>
> @@ -221,8 +222,10 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
>      sps->pic_struct_present_flag = get_bits1(gb);
>      if (!get_bits_left(gb))
>          return 0;
> +
>      sps->bitstream_restriction_flag = get_bits1(gb);
>      if (sps->bitstream_restriction_flag) {
> +        idx = gb->index;
>          get_bits1(gb);     /* motion_vectors_over_pic_boundaries_flag */
>          get_ue_golomb(gb); /* max_bytes_per_pic_denom */
>          get_ue_golomb(gb); /* max_bits_per_mb_denom */
> @@ -234,6 +237,7 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
>          if (get_bits_left(gb) < 0) {
>              sps->num_reorder_frames         = 0;
>              sps->bitstream_restriction_flag = 0;
> +            gb->index = idx;
>          }
>
>          if (sps->num_reorder_frames > 16U

This doesn't seem right. What guarantees are that when one element
can't be parsed, anything else can?

- Hendrik
Michael Niedermayer May 16, 2017, 8:46 p.m.
On Tue, May 16, 2017 at 03:13:36PM +0300, Ivan Shmakov wrote:
> ---
>  libavcodec/h264_ps.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 7858361..09e2290 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -136,6 +136,7 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
>  {
>      int aspect_ratio_info_present_flag;
>      unsigned int aspect_ratio_idc;
> +    int idx;
>  
>      aspect_ratio_info_present_flag = get_bits1(gb);
>  
> @@ -221,8 +222,10 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
>      sps->pic_struct_present_flag = get_bits1(gb);
>      if (!get_bits_left(gb))
>          return 0;
> +
>      sps->bitstream_restriction_flag = get_bits1(gb);
>      if (sps->bitstream_restriction_flag) {
> +        idx = gb->index;
>          get_bits1(gb);     /* motion_vectors_over_pic_boundaries_flag */
>          get_ue_golomb(gb); /* max_bytes_per_pic_denom */
>          get_ue_golomb(gb); /* max_bits_per_mb_denom */
> @@ -234,6 +237,7 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
>          if (get_bits_left(gb) < 0) {
>              sps->num_reorder_frames         = 0;
>              sps->bitstream_restriction_flag = 0;
> +            gb->index = idx;

Direct access to gb->index is not ok
all code directly accssing GetBitContext internals belongs in the
bitstream / get bits code.
Normally to "unread" we just duplicate the GetBitContext before


[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 7858361..09e2290 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -136,6 +136,7 @@  static inline int decode_vui_parameters(GetBitContext
*gb, AVCodecContext *avctx
 {
     int aspect_ratio_info_present_flag;
     unsigned int aspect_ratio_idc;
+    int idx;
 
     aspect_ratio_info_present_flag = get_bits1(gb);