Re: [FFmpeg-devel] [RFC] [PATCH] hevc_ps: reorder SPS reading to allow some errors
On Thu, Aug 21, 2014 at 10:07:23PM -0700, Timothy Gu wrote: On Aug 21, 2014 10:14 AM, Christophe Gisquet christophe.gisq...@gmail.com wrote: Hi, 2014-08-21 3:06 GMT+02:00 Michael Niedermayer michae...@gmx.at: heres a narrower solution for this file not sure its better i can apply whichever you prefer, probably we wont find out which is the best solution before pushing something and waiting for more bug reports Thanks for narrowing it done, and now I have dug it further. When you look at what get parsed before that, vui-def_disp_win.left_offset gets an insane value unrelated to the video dimensions. I tried instead to do as if vui-default_display_window_flag and related syntax does not exist. Then, the timing info is decoded, and the values are actually normal: vui_num_units_in_tick = 1000 vui_time_scale = 23976 (rest less relevant and meaningfull) So I think the actual bug in that stream is that it does not contain any default_display_window information. Why is that? Probably because the encoder writes an older VUI syntax, and several bits get misinterpreted. By luck they don't bother us. It would be nice to know what syntax is used, but there could be so many variations... The attached therefore barely improves on your patch. -int sar_present; +GetBitContext backup; +int sar_present, alt = 1; Shouldn't alt be initialized to 0? fixed thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC] [PATCH] hevc_ps: reorder SPS reading to allow some errors
Hi,
2014-08-21 3:06 GMT+02:00 Michael Niedermayer michae...@gmx.at:
heres a narrower solution for this file
not sure its better
i can apply whichever you prefer, probably we wont find out which
is the best solution before pushing something and waiting for more
bug reports
Thanks for narrowing it done, and now I have dug it further.
When you look at what get parsed before that,
vui-def_disp_win.left_offset gets an insane value unrelated to the
video dimensions. I tried instead to do as if
vui-default_display_window_flag and related syntax does not exist.
Then, the timing info is decoded, and the values are actually normal:
vui_num_units_in_tick = 1000
vui_time_scale = 23976
(rest less relevant and meaningfull)
So I think the actual bug in that stream is that it does not contain
any default_display_window information. Why is that? Probably because
the encoder writes an older VUI syntax, and several bits get
misinterpreted. By luck they don't bother us. It would be nice to know
what syntax is used, but there could be so many variations...
The attached therefore barely improves on your patch.
--
Christophe
From 80cd353f464f3785285e0c82efd3c6ccd655bf5c Mon Sep 17 00:00:00 2001
From: Christophe Gisquet christophe.gisq...@gmail.com
Date: Thu, 21 Aug 2014 18:57:18 +0200
Subject: [PATCH] hevc_ps: check overflow and test alternate syntax
Some streams were found to have what appeared to be truncated SPS.
Their syntax seem to be valid at least until the end of the VUI, so
try that syntax if the parsing would overflow the SPS in the
conforming syntax.
Fixes ticket #3872.
---
libavcodec/hevc_ps.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index 2ccce5f..29412d2 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -471,7 +471,8 @@ static void decode_vui(HEVCContext *s, HEVCSPS *sps)
{
VUI *vui = sps-vui;
GetBitContext *gb = s-HEVClc-gb;
-int sar_present;
+GetBitContext backup;
+int sar_present, alt = 1;
av_log(s-avctx, AV_LOG_DEBUG, Decoding VUI\n);
@@ -525,6 +526,10 @@ static void decode_vui(HEVCContext *s, HEVCSPS *sps)
vui-frame_field_info_present_flag = get_bits1(gb);
vui-default_display_window_flag = get_bits1(gb);
+// Backup context in case an alternate header is detected
+if( get_bits_left(gb) = 66)
+memcpy(backup, gb, sizeof(backup));
+
if (vui-default_display_window_flag) {
//TODO: * 2 is only valid for 420
vui-def_disp_win.left_offset = get_ue_golomb_long(gb) * 2;
@@ -552,8 +557,22 @@ static void decode_vui(HEVCContext *s, HEVCSPS *sps)
vui-vui_timing_info_present_flag = get_bits1(gb);
if (vui-vui_timing_info_present_flag) {
+if( get_bits_left(gb) 66) {
+// The alternate syntax seem to have timing info located
+// at where def_disp_win is normally located
+av_log(s-avctx, AV_LOG_WARNING,
+ Strange VUI timing information, retrying...\n);
+vui-default_display_window_flag = 0;
+memset(vui-def_disp_win, 0, sizeof(vui-def_disp_win));
+memcpy(gb, backup, sizeof(backup));
+alt = 1;
+}
vui-vui_num_units_in_tick = get_bits_long(gb, 32);
vui-vui_time_scale = get_bits_long(gb, 32);
+if (alt) {
+av_log(s-avctx, AV_LOG_INFO, Retry got %i/%i fps\n,
+ vui-vui_time_scale, vui-vui_num_units_in_tick);
+}
vui-vui_poc_proportional_to_timing_flag = get_bits1(gb);
if (vui-vui_poc_proportional_to_timing_flag)
vui-vui_num_ticks_poc_diff_one_minus1 = get_ue_golomb_long(gb);
--
1.9.2.msysgit.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC] [PATCH] hevc_ps: reorder SPS reading to allow some errors
On Thu, Aug 21, 2014 at 07:14:03PM +0200, Christophe Gisquet wrote: Hi, 2014-08-21 3:06 GMT+02:00 Michael Niedermayer michae...@gmx.at: heres a narrower solution for this file not sure its better i can apply whichever you prefer, probably we wont find out which is the best solution before pushing something and waiting for more bug reports Thanks for narrowing it done, and now I have dug it further. When you look at what get parsed before that, vui-def_disp_win.left_offset gets an insane value unrelated to the video dimensions. I tried instead to do as if vui-default_display_window_flag and related syntax does not exist. Then, the timing info is decoded, and the values are actually normal: vui_num_units_in_tick = 1000 vui_time_scale = 23976 (rest less relevant and meaningfull) So I think the actual bug in that stream is that it does not contain any default_display_window information. Why is that? Probably because the encoder writes an older VUI syntax, and several bits get misinterpreted. By luck they don't bother us. It would be nice to know what syntax is used, but there could be so many variations... The attached therefore barely improves on your patch. -- Christophe hevc_ps.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) b60bac9ae4f56940b8e219dbe735308c38b9a57a 0001-hevc_ps-check-overflow-and-test-alternate-syntax.patch From 80cd353f464f3785285e0c82efd3c6ccd655bf5c Mon Sep 17 00:00:00 2001 From: Christophe Gisquet christophe.gisq...@gmail.com Date: Thu, 21 Aug 2014 18:57:18 +0200 Subject: [PATCH] hevc_ps: check overflow and test alternate syntax Some streams were found to have what appeared to be truncated SPS. Their syntax seem to be valid at least until the end of the VUI, so try that syntax if the parsing would overflow the SPS in the conforming syntax. Fixes ticket #3872. applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC] [PATCH] hevc_ps: reorder SPS reading to allow some errors
On Aug 21, 2014 10:14 AM, Christophe Gisquet christophe.gisq...@gmail.com wrote: Hi, 2014-08-21 3:06 GMT+02:00 Michael Niedermayer michae...@gmx.at: heres a narrower solution for this file not sure its better i can apply whichever you prefer, probably we wont find out which is the best solution before pushing something and waiting for more bug reports Thanks for narrowing it done, and now I have dug it further. When you look at what get parsed before that, vui-def_disp_win.left_offset gets an insane value unrelated to the video dimensions. I tried instead to do as if vui-default_display_window_flag and related syntax does not exist. Then, the timing info is decoded, and the values are actually normal: vui_num_units_in_tick = 1000 vui_time_scale = 23976 (rest less relevant and meaningfull) So I think the actual bug in that stream is that it does not contain any default_display_window information. Why is that? Probably because the encoder writes an older VUI syntax, and several bits get misinterpreted. By luck they don't bother us. It would be nice to know what syntax is used, but there could be so many variations... The attached therefore barely improves on your patch. -int sar_present; +GetBitContext backup; +int sar_present, alt = 1; Shouldn't alt be initialized to 0? Sorry about being too late. Timothy ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
