[FFmpeg-devel] [PATCH] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[FFmpeg-devel] [PATCH] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[1035567130]

From: Wang Yaqiang <wangyaqiang03@kuaishou.com>

In some videos, SPS will be stored before VPS in hvcC box,
parse SPS does not depend on VPS, so the video is expected to be processed normally.
Added "parsed_vps" parameter to indicate whether VPS have been parsed.
Only VPS have been parsed can be verified during SPS parsing.

Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
---
 libavcodec/hevc_ps.c | 4 ++--
 libavcodec/hevc_ps.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index f665d8053c..71835d0f36 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -464,7 +464,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx,
     if (!vps_buf)
         return AVERROR(ENOMEM);
     vps = (HEVCVPS*)vps_buf->data;
-
+    ps->parsed_vps = 1;
     av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n");
 
     nal_size = gb->buffer_end - gb->buffer;
@@ -1272,7 +1272,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx,
 
     ret = ff_hevc_parse_sps(sps, gb, &sps_id,
                             apply_defdispwin,
-                            ps->vps_list, avctx);
+                            ps->parsed_vps ? ps->vps_list : NULL, avctx);
     if (ret < 0) {
         av_buffer_unref(&sps_buf);
         return ret;
diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h
index 2a1bbf6489..e6b694e7f3 100644
--- a/libavcodec/hevc_ps.h
+++ b/libavcodec/hevc_ps.h
@@ -333,6 +333,7 @@ typedef struct HEVCParamSets {
     const HEVCVPS *vps;
     const HEVCSPS *sps;
     const HEVCPPS *pps;
+    int parsed_vps; // indicates VPS has been parsed
 } HEVCParamSets;
 
 /**
-- 
2.33.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[James Almer]

On 9/9/2022 10:09 AM, 1035567130@qq.com wrote:
> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> 
> In some videos, SPS will be stored before VPS in hvcC box,
> parse SPS does not depend on VPS, so the video is expected to be processed normally.
> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
> Only VPS have been parsed can be verified during SPS parsing.
> 
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
>   libavcodec/hevc_ps.c | 4 ++--
>   libavcodec/hevc_ps.h | 1 +
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index f665d8053c..71835d0f36 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -464,7 +464,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx,
>       if (!vps_buf)
>           return AVERROR(ENOMEM);
>       vps = (HEVCVPS*)vps_buf->data;
> -
> +    ps->parsed_vps = 1;

This would need to be reset in ff_hevc_ps_uninit().

>       av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n");
>   
>       nal_size = gb->buffer_end - gb->buffer;
> @@ -1272,7 +1272,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx,
>   
>       ret = ff_hevc_parse_sps(sps, gb, &sps_id,
>                               apply_defdispwin,
> -                            ps->vps_list, avctx);
> +                            ps->parsed_vps ? ps->vps_list : NULL, avctx);

If VPS is not required to parse SPS, wouldn't it be better to just 
change the check in ff_hevc_parse_sps() to turn the log message into a 
warning and then not error out?

>       if (ret < 0) {
>           av_buffer_unref(&sps_buf);
>           return ret;
> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h
> index 2a1bbf6489..e6b694e7f3 100644
> --- a/libavcodec/hevc_ps.h
> +++ b/libavcodec/hevc_ps.h
> @@ -333,6 +333,7 @@ typedef struct HEVCParamSets {
>       const HEVCVPS *vps;
>       const HEVCSPS *sps;
>       const HEVCPPS *pps;
> +    int parsed_vps; // indicates VPS has been parsed
>   } HEVCParamSets;
>   
>   /**
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[wangyaqiang]

> 2022年9月9日 下午9:55，James Almer <jamrial@gmail.com> 写道：
> 
> On 9/9/2022 10:09 AM, 1035567130@qq.com <mailto:1035567130@qq.com> wrote:
>> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>> In some videos, SPS will be stored before VPS in hvcC box,
>> parse SPS does not depend on VPS, so the video is expected to be processed normally.
>> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
>> Only VPS have been parsed can be verified during SPS parsing.
>> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>> ---
>>  libavcodec/hevc_ps.c | 4 ++--
>>  libavcodec/hevc_ps.h | 1 +
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
>> index f665d8053c..71835d0f36 100644
>> --- a/libavcodec/hevc_ps.c
>> +++ b/libavcodec/hevc_ps.c
>> @@ -464,7 +464,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx,
>>      if (!vps_buf)
>>          return AVERROR(ENOMEM);
>>      vps = (HEVCVPS*)vps_buf->data;
>> -
>> +    ps->parsed_vps = 1;
> 
> This would need to be reset in ff_hevc_ps_uninit().

Thanks，I will fix it in the next commit.

> 
>>      av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n");
>>        nal_size = gb->buffer_end - gb->buffer;
>> @@ -1272,7 +1272,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx,
>>        ret = ff_hevc_parse_sps(sps, gb, &sps_id,
>>                              apply_defdispwin,
>> -                            ps->vps_list, avctx);
>> +                            ps->parsed_vps ? ps->vps_list : NULL, avctx);
> 
> If VPS is not required to parse SPS, wouldn't it be better to just change the check in ff_hevc_parse_sps() to turn the log message into a warning and then not error out?

The SPS contains the vps_id,in the original logic,check the vps_id based on the vps_list param，so the original logic is preserved. i’m not sure which way is better.

> 
>>      if (ret < 0) {
>>          av_buffer_unref(&sps_buf);
>>          return ret;
>> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h
>> index 2a1bbf6489..e6b694e7f3 100644
>> --- a/libavcodec/hevc_ps.h
>> +++ b/libavcodec/hevc_ps.h
>> @@ -333,6 +333,7 @@ typedef struct HEVCParamSets {
>>      const HEVCVPS *vps;
>>      const HEVCSPS *sps;
>>      const HEVCPPS *pps;
>> +    int parsed_vps; // indicates VPS has been parsed
>>  } HEVCParamSets;
>>    /**
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

_______________________________________________
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".

[FFmpeg-devel] [PATCH v2] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[1035567130]

From: Wang Yaqiang <wangyaqiang03@kuaishou.com>

In some videos, SPS will be stored before VPS in hvcC box,
parse SPS does not depend on VPS, so the video is expected to be processed normally.
Added "parsed_vps" parameter to indicate whether VPS have been parsed.
Only VPS have been parsed can be verified during SPS parsing.

Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
---
 libavcodec/hevc_ps.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index f665d8053c..9abee8bd90 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -916,9 +916,8 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id,
     sps->vps_id = get_bits(gb, 4);
 
     if (vps_list && !vps_list[sps->vps_id]) {
-        av_log(avctx, AV_LOG_ERROR, "VPS %d does not exist\n",
+        av_log(avctx, AV_LOG_WARNING, "VPS %d does not exist\n",
                sps->vps_id);
-        return AVERROR_INVALIDDATA;
     }
 
     sps->max_sub_layers = get_bits(gb, 3) + 1;
-- 
2.33.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1452 bytes --]

On Mon, Sep 26, 2022 at 05:38:14PM +0800, 1035567130@qq.com wrote:
> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> 
> In some videos, SPS will be stored before VPS in hvcC box,
> parse SPS does not depend on VPS, so the video is expected to be processed normally.
> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
> Only VPS have been parsed can be verified during SPS parsing.
> 
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
>  libavcodec/hevc_ps.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

This causes segfaults

==816== Invalid read of size 8
==816==    at 0xFAF178: hevc_parse (in ffmpeg_g)
==816==    by 0xA7A2A6: av_parser_parse2 (in ffmpeg_g)
==816==    by 0x5FC388: parse_packet (in ffmpeg_g)
==816==    by 0x5FDC5D: read_frame_internal (in ffmpeg_g)
==816==    by 0x5FFA10: avformat_find_stream_info (in ffmpeg_g)
==816==    by 0x2F6054: open_input_file (in ffmpeg_g)
==816==    by 0x2FC6AB: ffmpeg_parse_options (in ffmpeg_g)
==816==    by 0x2E8A34: main (in ffmpeg_g)
==816==  Address 0x8 is not stack'd, malloc'd or (recently) free'd


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[wangyaqiang]

> 2022年9月27日 04:21，Michael Niedermayer <michael@niedermayer.cc> 写道：
> 
> On Mon, Sep 26, 2022 at 05:38:14PM +0800, 1035567130@qq.com wrote:
>> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>> 
>> In some videos, SPS will be stored before VPS in hvcC box,
>> parse SPS does not depend on VPS, so the video is expected to be processed normally.
>> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
>> Only VPS have been parsed can be verified during SPS parsing.
>> 
>> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>> ---
>> libavcodec/hevc_ps.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> This causes segfaults
> 
> ==816== Invalid read of size 8
> ==816==    at 0xFAF178: hevc_parse (in ffmpeg_g)
> ==816==    by 0xA7A2A6: av_parser_parse2 (in ffmpeg_g)
> ==816==    by 0x5FC388: parse_packet (in ffmpeg_g)
> ==816==    by 0x5FDC5D: read_frame_internal (in ffmpeg_g)
> ==816==    by 0x5FFA10: avformat_find_stream_info (in ffmpeg_g)
> ==816==    by 0x2F6054: open_input_file (in ffmpeg_g)
> ==816==    by 0x2FC6AB: ffmpeg_parse_options (in ffmpeg_g)
> ==816==    by 0x2E8A34: main (in ffmpeg_g)
> ==816==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
> 
> 
> [...]
> 

This is a serious problem, but I haven't occur it. Could you please provide with test materials or test methods? Thanks

> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> When the tyrant has disposed of foreign enemies by conquest or treaty, and
> there is nothing more to fear from them, then he is always stirring up
> some war or other, in order that the people may require a leader. -- Plato
> _______________________________________________
> 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".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[wangyaqiang]

> 2022年9月27日 04:21，Michael Niedermayer <michael@niedermayer.cc> 写道：
> 
> On Mon, Sep 26, 2022 at 05:38:14PM +0800, 1035567130@qq.com wrote:
>> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>> 
>> In some videos, SPS will be stored before VPS in hvcC box,
>> parse SPS does not depend on VPS, so the video is expected to be processed normally.
>> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
>> Only VPS have been parsed can be verified during SPS parsing.
>> 
>> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>> ---
>> libavcodec/hevc_ps.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> This causes segfaults
> 
> ==816== Invalid read of size 8
> ==816==    at 0xFAF178: hevc_parse (in ffmpeg_g)
> ==816==    by 0xA7A2A6: av_parser_parse2 (in ffmpeg_g)
> ==816==    by 0x5FC388: parse_packet (in ffmpeg_g)
> ==816==    by 0x5FDC5D: read_frame_internal (in ffmpeg_g)
> ==816==    by 0x5FFA10: avformat_find_stream_info (in ffmpeg_g)
> ==816==    by 0x2F6054: open_input_file (in ffmpeg_g)
> ==816==    by 0x2FC6AB: ffmpeg_parse_options (in ffmpeg_g)
> ==816==    by 0x2E8A34: main (in ffmpeg_g)
> ==816==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
> 
> 
> [...]

Excuse me, we have run tests on our own business and have not found this problem, but it is a hidden risk,really hope you can tell me how to reproduce this problem. Thanks

> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> When the tyrant has disposed of foreign enemies by conquest or treaty, and
> there is nothing more to fear from them, then he is always stirring up
> some war or other, in order that the people may require a leader. -- Plato
> _______________________________________________
> 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".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 4713 bytes --]

On Fri, Oct 14, 2022 at 06:13:14PM +0800, wangyaqiang wrote:
> 
> 
> > 2022年9月27日 04:21，Michael Niedermayer <michael@niedermayer.cc> 写道：
> > 
> > On Mon, Sep 26, 2022 at 05:38:14PM +0800, 1035567130@qq.com wrote:
> >> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> >> 
> >> In some videos, SPS will be stored before VPS in hvcC box,
> >> parse SPS does not depend on VPS, so the video is expected to be processed normally.
> >> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
> >> Only VPS have been parsed can be verified during SPS parsing.
> >> 
> >> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> >> ---
> >> libavcodec/hevc_ps.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > This causes segfaults
> > 
> > ==816== Invalid read of size 8
> > ==816==    at 0xFAF178: hevc_parse (in ffmpeg_g)
> > ==816==    by 0xA7A2A6: av_parser_parse2 (in ffmpeg_g)
> > ==816==    by 0x5FC388: parse_packet (in ffmpeg_g)
> > ==816==    by 0x5FDC5D: read_frame_internal (in ffmpeg_g)
> > ==816==    by 0x5FFA10: avformat_find_stream_info (in ffmpeg_g)
> > ==816==    by 0x2F6054: open_input_file (in ffmpeg_g)
> > ==816==    by 0x2FC6AB: ffmpeg_parse_options (in ffmpeg_g)
> > ==816==    by 0x2E8A34: main (in ffmpeg_g)
> > ==816==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
> > 
> > 
> > [...]
> 
> Excuse me, we have run tests on our own business and have not found this problem, but it is a hidden risk,really hope you can tell me how to reproduce this problem. Thanks

heres a more complete stack trace, i willmail you the input sample privatly

Trailing option(s) found in the command: may be ignored.
[hevc @ 0x16a0ad40] Invalid NAL unit 0, skipping.
[hevc @ 0x16a0ad40] PTL information too short
==24589==    at 0x12A19DF: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
==24589==    by 0x12A259D: av_log_default_callback (log.c:399)
==24589==    by 0x12A2844: av_vlog (log.c:434)
==24589==    by 0x12A26A3: av_log (log.c:413)
==24589==    by 0x10A7216: parse_ptl (hevc_ps.c:342)
==24589==    by 0x10A78B4: ff_hevc_decode_nal_vps (hevc_ps.c:503)
==24589==    by 0x10A579C: parse_nal_units (hevc_parser.c:212)
==24589==    by 0x10A5B46: hevc_parse (hevc_parser.c:331)
==24589==    by 0xB82366: av_parser_parse2 (parser.c:163)
==24589==    by 0x61BD17: parse_packet (demux.c:1140)
==24589==    by 0x61C936: read_frame_internal (demux.c:1334)
==24589==    by 0x6217A7: avformat_find_stream_info (demux.c:2612)
==24589==    by 0x246A51: open_input_file (ffmpeg_opt.c:1315)
==24589==    by 0x255E38: open_files (ffmpeg_opt.c:3703)
==24589==    by 0x255FEC: ffmpeg_parse_options (ffmpeg_opt.c:3742)
==24589==    by 0x26EFEE: main (ffmpeg.c:4236)
[hevc @ 0x16a0ad40] VPS 0 does not exist
==24589== Invalid read of size 8
==24589==    at 0x10A5189: hevc_parse_slice_header (hevc_parser.c:88)
==24589==    by 0x10A584E: parse_nal_units (hevc_parser.c:245)
==24589==    by 0x10A5B46: hevc_parse (hevc_parser.c:331)
==24589==    by 0xB82366: av_parser_parse2 (parser.c:163)
==24589==    by 0x61BD17: parse_packet (demux.c:1140)
==24589==    by 0x61C936: read_frame_internal (demux.c:1334)
==24589==    by 0x6217A7: avformat_find_stream_info (demux.c:2612)
==24589==    by 0x246A51: open_input_file (ffmpeg_opt.c:1315)
==24589==    by 0x255E38: open_files (ffmpeg_opt.c:3703)
==24589==    by 0x255FEC: ffmpeg_parse_options (ffmpeg_opt.c:3742)
==24589==    by 0x26EFEE: main (ffmpeg.c:4236)
==24589==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==24589== 
==24589== 
==24589== Process terminating with default action of signal 11 (SIGSEGV)
==24589==  Access not within mapped region at address 0x8
==24589==    at 0x10A5189: hevc_parse_slice_header (hevc_parser.c:88)
==24589==    by 0x10A584E: parse_nal_units (hevc_parser.c:245)
==24589==    by 0x10A5B46: hevc_parse (hevc_parser.c:331)
==24589==    by 0xB82366: av_parser_parse2 (parser.c:163)
==24589==    by 0x61BD17: parse_packet (demux.c:1140)
==24589==    by 0x61C936: read_frame_internal (demux.c:1334)
==24589==    by 0x6217A7: avformat_find_stream_info (demux.c:2612)
==24589==    by 0x246A51: open_input_file (ffmpeg_opt.c:1315)
==24589==    by 0x255E38: open_files (ffmpeg_opt.c:3703)
==24589==    by 0x255FEC: ffmpeg_parse_options (ffmpeg_opt.c:3742)
==24589==    by 0x26EFEE: main (ffmpeg.c:4236)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[wangyaqiang]

> 2022年10月15日 07:00，Michael Niedermayer <michael@niedermayer.cc> 写道：
> 
> On Fri, Oct 14, 2022 at 06:13:14PM +0800, wangyaqiang wrote:
>> 
>> 
>>> 2022年9月27日 04:21，Michael Niedermayer <michael@niedermayer.cc> 写道：
>>> 
>>> On Mon, Sep 26, 2022 at 05:38:14PM +0800, 1035567130@qq.com wrote:
>>>> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>>>> 
>>>> In some videos, SPS will be stored before VPS in hvcC box,
>>>> parse SPS does not depend on VPS, so the video is expected to be processed normally.
>>>> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
>>>> Only VPS have been parsed can be verified during SPS parsing.
>>>> 
>>>> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>>>> ---
>>>> libavcodec/hevc_ps.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>> 
>>> This causes segfaults
>>> 
>>> ==816== Invalid read of size 8
>>> ==816==    at 0xFAF178: hevc_parse (in ffmpeg_g)
>>> ==816==    by 0xA7A2A6: av_parser_parse2 (in ffmpeg_g)
>>> ==816==    by 0x5FC388: parse_packet (in ffmpeg_g)
>>> ==816==    by 0x5FDC5D: read_frame_internal (in ffmpeg_g)
>>> ==816==    by 0x5FFA10: avformat_find_stream_info (in ffmpeg_g)
>>> ==816==    by 0x2F6054: open_input_file (in ffmpeg_g)
>>> ==816==    by 0x2FC6AB: ffmpeg_parse_options (in ffmpeg_g)
>>> ==816==    by 0x2E8A34: main (in ffmpeg_g)
>>> ==816==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
>>> 
>>> 
>>> [...]
>> 
>> Excuse me, we have run tests on our own business and have not found this problem, but it is a hidden risk,really hope you can tell me how to reproduce this problem. Thanks
> 
> heres a more complete stack trace, i willmail you the input sample privatly
> 

Thanks very much for the materials. Under the condition that vps is not strongly checked when parsing sps, it is necessary to determine whether vps is empty when vps is used. I submitted a new patch to fix the crash, but I feel that this way is a little trick,any good ideas?

> Trailing option(s) found in the command: may be ignored.
> [hevc @ 0x16a0ad40] Invalid NAL unit 0, skipping.
> [hevc @ 0x16a0ad40] PTL information too short
> ==24589==    at 0x12A19DF: VALGRIND_PRINTF_BACKTRACE (valgrind.h:6303)
> ==24589==    by 0x12A259D: av_log_default_callback (log.c:399)
> ==24589==    by 0x12A2844: av_vlog (log.c:434)
> ==24589==    by 0x12A26A3: av_log (log.c:413)
> ==24589==    by 0x10A7216: parse_ptl (hevc_ps.c:342)
> ==24589==    by 0x10A78B4: ff_hevc_decode_nal_vps (hevc_ps.c:503)
> ==24589==    by 0x10A579C: parse_nal_units (hevc_parser.c:212)
> ==24589==    by 0x10A5B46: hevc_parse (hevc_parser.c:331)
> ==24589==    by 0xB82366: av_parser_parse2 (parser.c:163)
> ==24589==    by 0x61BD17: parse_packet (demux.c:1140)
> ==24589==    by 0x61C936: read_frame_internal (demux.c:1334)
> ==24589==    by 0x6217A7: avformat_find_stream_info (demux.c:2612)
> ==24589==    by 0x246A51: open_input_file (ffmpeg_opt.c:1315)
> ==24589==    by 0x255E38: open_files (ffmpeg_opt.c:3703)
> ==24589==    by 0x255FEC: ffmpeg_parse_options (ffmpeg_opt.c:3742)
> ==24589==    by 0x26EFEE: main (ffmpeg.c:4236)
> [hevc @ 0x16a0ad40] VPS 0 does not exist
> ==24589== Invalid read of size 8
> ==24589==    at 0x10A5189: hevc_parse_slice_header (hevc_parser.c:88)
> ==24589==    by 0x10A584E: parse_nal_units (hevc_parser.c:245)
> ==24589==    by 0x10A5B46: hevc_parse (hevc_parser.c:331)
> ==24589==    by 0xB82366: av_parser_parse2 (parser.c:163)
> ==24589==    by 0x61BD17: parse_packet (demux.c:1140)
> ==24589==    by 0x61C936: read_frame_internal (demux.c:1334)
> ==24589==    by 0x6217A7: avformat_find_stream_info (demux.c:2612)
> ==24589==    by 0x246A51: open_input_file (ffmpeg_opt.c:1315)
> ==24589==    by 0x255E38: open_files (ffmpeg_opt.c:3703)
> ==24589==    by 0x255FEC: ffmpeg_parse_options (ffmpeg_opt.c:3742)
> ==24589==    by 0x26EFEE: main (ffmpeg.c:4236)
> ==24589==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
> ==24589== 
> ==24589== 
> ==24589== Process terminating with default action of signal 11 (SIGSEGV)
> ==24589==  Access not within mapped region at address 0x8
> ==24589==    at 0x10A5189: hevc_parse_slice_header (hevc_parser.c:88)
> ==24589==    by 0x10A584E: parse_nal_units (hevc_parser.c:245)
> ==24589==    by 0x10A5B46: hevc_parse (hevc_parser.c:331)
> ==24589==    by 0xB82366: av_parser_parse2 (parser.c:163)
> ==24589==    by 0x61BD17: parse_packet (demux.c:1140)
> ==24589==    by 0x61C936: read_frame_internal (demux.c:1334)
> ==24589==    by 0x6217A7: avformat_find_stream_info (demux.c:2612)
> ==24589==    by 0x246A51: open_input_file (ffmpeg_opt.c:1315)
> ==24589==    by 0x255E38: open_files (ffmpeg_opt.c:3703)
> ==24589==    by 0x255FEC: ffmpeg_parse_options (ffmpeg_opt.c:3742)
> ==24589==    by 0x26EFEE: main (ffmpeg.c:4236)
> 
> [...]
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Concerning the gods, I have no means of knowing whether they exist or not
> or of what sort they may be, because of the obscurity of the subject, and
> the brevity of human life -- Protagoras
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

_______________________________________________
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".

[FFmpeg-devel] [PATCH v3] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[1035567130]

From: Wang Yaqiang <wangyaqiang03@kuaishou.com>

In some videos, SPS will be stored before VPS in hvcC box,
parse SPS does not depend on VPS, so the video is expected to be processed normally.
Added "parsed_vps" parameter to indicate whether VPS have been parsed.
Only VPS have been parsed can be verified during SPS parsing.

Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
---
 libavcodec/hevc_parser.c | 8 ++++++++
 libavcodec/hevc_ps.c     | 3 +--
 libavcodec/hevcdec.c     | 9 ++++++++-
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
index 59f9a0ff3e..be4aa55e51 100644
--- a/libavcodec/hevc_parser.c
+++ b/libavcodec/hevc_parser.c
@@ -85,8 +85,16 @@ static int hevc_parse_slice_header(AVCodecParserContext *s, H2645NAL *nal,
     }
     if (ps->sps != (HEVCSPS*)ps->sps_list[ps->pps->sps_id]->data) {
         ps->sps = (HEVCSPS*)ps->sps_list[ps->pps->sps_id]->data;
+        if (ps->sps->vps_id >= HEVC_MAX_VPS_COUNT || !ps->vps_list[ps->sps->vps_id]) {
+            av_log(avctx, AV_LOG_ERROR, "VPS id out of range: %d\n", ps->sps->vps_id);
+            return AVERROR_INVALIDDATA;
+        }
         ps->vps = (HEVCVPS*)ps->vps_list[ps->sps->vps_id]->data;
     }
+    if (!ps->vps) {
+        av_log(avctx, AV_LOG_ERROR, "VPS %d does not exist\n", ps->sps->vps_id);
+        return AVERROR_INVALIDDATA;
+    }
     ow  = &ps->sps->output_window;
 
     s->coded_width  = ps->sps->width;
diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index f665d8053c..9abee8bd90 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -916,9 +916,8 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id,
     sps->vps_id = get_bits(gb, 4);
 
     if (vps_list && !vps_list[sps->vps_id]) {
-        av_log(avctx, AV_LOG_ERROR, "VPS %d does not exist\n",
+        av_log(avctx, AV_LOG_WARNING, "VPS %d does not exist\n",
                sps->vps_id);
-        return AVERROR_INVALIDDATA;
     }
 
     sps->max_sub_layers = get_bits(gb, 3) + 1;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index fb44d8d3f2..03942a150d 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -327,6 +327,10 @@ static void export_stream_params(HEVCContext *s, const HEVCSPS *sps)
 {
     AVCodecContext *avctx = s->avctx;
     const HEVCParamSets *ps = &s->ps;
+    if (!s->ps.vps_list[sps->vps_id]) {
+        av_log(avctx, AV_LOG_ERROR, "VPS id out of range: %d\n", sps->vps_id);
+        return;
+    }
     const HEVCVPS *vps = (const HEVCVPS*)ps->vps_list[sps->vps_id]->data;
     const HEVCWindow *ow = &sps->output_window;
     unsigned int num = 0, den = 0;
@@ -520,7 +524,10 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps,
     ret = pic_arrays_init(s, sps);
     if (ret < 0)
         goto fail;
-
+    if (!s->ps.vps_list[sps->vps_id]) {
+        ret = AVERROR_INVALIDDATA;
+        goto fail;
+    }
     export_stream_params(s, sps);
 
     s->avctx->pix_fmt = pix_fmt;
-- 
2.33.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v3] lavc/hevc_ps: fix process failed when SPS before VPS in hvcC

[wangyaqiang]

Ping

> 2022年10月26日 10:23，1035567130@qq.com 写道：
> 
> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> 
> In some videos, SPS will be stored before VPS in hvcC box,
> parse SPS does not depend on VPS, so the video is expected to be processed normally.
> Added "parsed_vps" parameter to indicate whether VPS have been parsed.
> Only VPS have been parsed can be verified during SPS parsing.
> 
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
> libavcodec/hevc_parser.c | 8 ++++++++
> libavcodec/hevc_ps.c     | 3 +--
> libavcodec/hevcdec.c     | 9 ++++++++-
> 3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> index 59f9a0ff3e..be4aa55e51 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -85,8 +85,16 @@ static int hevc_parse_slice_header(AVCodecParserContext *s, H2645NAL *nal,
>     }
>     if (ps->sps != (HEVCSPS*)ps->sps_list[ps->pps->sps_id]->data) {
>         ps->sps = (HEVCSPS*)ps->sps_list[ps->pps->sps_id]->data;
> +        if (ps->sps->vps_id >= HEVC_MAX_VPS_COUNT || !ps->vps_list[ps->sps->vps_id]) {
> +            av_log(avctx, AV_LOG_ERROR, "VPS id out of range: %d\n", ps->sps->vps_id);
> +            return AVERROR_INVALIDDATA;
> +        }
>         ps->vps = (HEVCVPS*)ps->vps_list[ps->sps->vps_id]->data;
>     }
> +    if (!ps->vps) {
> +        av_log(avctx, AV_LOG_ERROR, "VPS %d does not exist\n", ps->sps->vps_id);
> +        return AVERROR_INVALIDDATA;
> +    }
>     ow  = &ps->sps->output_window;
> 
>     s->coded_width  = ps->sps->width;
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index f665d8053c..9abee8bd90 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -916,9 +916,8 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id,
>     sps->vps_id = get_bits(gb, 4);
> 
>     if (vps_list && !vps_list[sps->vps_id]) {
> -        av_log(avctx, AV_LOG_ERROR, "VPS %d does not exist\n",
> +        av_log(avctx, AV_LOG_WARNING, "VPS %d does not exist\n",
>                sps->vps_id);
> -        return AVERROR_INVALIDDATA;
>     }
> 
>     sps->max_sub_layers = get_bits(gb, 3) + 1;
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index fb44d8d3f2..03942a150d 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -327,6 +327,10 @@ static void export_stream_params(HEVCContext *s, const HEVCSPS *sps)
> {
>     AVCodecContext *avctx = s->avctx;
>     const HEVCParamSets *ps = &s->ps;
> +    if (!s->ps.vps_list[sps->vps_id]) {
> +        av_log(avctx, AV_LOG_ERROR, "VPS id out of range: %d\n", sps->vps_id);
> +        return;
> +    }
>     const HEVCVPS *vps = (const HEVCVPS*)ps->vps_list[sps->vps_id]->data;
>     const HEVCWindow *ow = &sps->output_window;
>     unsigned int num = 0, den = 0;
> @@ -520,7 +524,10 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps,
>     ret = pic_arrays_init(s, sps);
>     if (ret < 0)
>         goto fail;
> -
> +    if (!s->ps.vps_list[sps->vps_id]) {
> +        ret = AVERROR_INVALIDDATA;
> +        goto fail;
> +    }
>     export_stream_params(s, sps);
> 
>     s->avctx->pix_fmt = pix_fmt;
> -- 
> 2.33.0
> 

_______________________________________________
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".