On 1/29/2024 5:19 PM, Frank Plowman wrote:
On 29/01/2024 19:04, James Almer wrote:
Well, turns out the current code is fine and my suggested change above
is wrong. Fun how that goes.
Can you test the following instead?
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..30b4ae3bc0 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -764,7 +764,7 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
infer(vps_each_layer_is_an_ols_flag, 0);
if (!current->vps_each_layer_is_an_ols_flag) {
if (!current->vps_all_independent_layers_flag)
- ub(2, vps_ols_mode_idc);
+ u(2, vps_ols_mode_idc, 0, 2);
else
infer(vps_ols_mode_idc, 2);
if (current->vps_ols_mode_idc == 2) {
The spec reads "Decoders conforming to this version of this
Specification shall *ignore* the OLSs with
vps_ols_mode_idc equal to 3." This change throws an error for these
OLSs, which I don't think is correct.
There is already some logic just below this to warn the user if
vps_ols_mode_idc is 3.
@@ -902,11 +902,10 @@ static int FUNC(vps) (CodedBitstreamContext
*ctx, RWContext *rw,
current->vps_ols_mode_idc == 1) {
num_layers_in_ols = i + 1;
} else if (current->vps_ols_mode_idc == 2) {
- for (k = 0, j = 0; k <=
current->vps_max_layers_minus1; k++) {
+ for (k = 0, j = 0; k <=
current->vps_max_layers_minus1; k++)
if (layer_included_in_ols_flag[i][k])
j++;
- num_layers_in_ols = j;
- }
+ num_layers_in_ols = j;
}
if (num_layers_in_ols > 1) {
num_multi_layer_olss++;
This looks good to me, the old behaviour was wrong. I don't think this
is what was causing this
particular crash however.
Will apply this part then.
Below is a patch which addresses the issue, an integer overflow when
calculating the bounds for
vps_num_ols_timing_hrd_params_minus1. There's also a similar fix for
vps_num_dpb_params_minus1.
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..49bf2e45ac 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -946,7 +946,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
if (!current->vps_each_layer_is_an_ols_flag) {
uint16_t vps_num_dpb_params;
- ue(vps_num_dpb_params_minus1, 0, num_multi_layer_olss - 1);
+ ue(vps_num_dpb_params_minus1, 0,
+ num_multi_layer_olss > 0 ? num_multi_layer_olss - 1 : 0);
FFMAX(0, num_multi_layer_olss - 1); looks better.
If the spec explicitly states num_multi_layer_olss - 1 should be used
here, wouldn't that mean that it doesn't expect num_multi_layer_olss to
be 0 at this point for vps_ols_mode_idc >= 0 && vps_ols_mode_idc < 3?
When vps_each_layer_is_an_ols_flag is true, num_multi_layer_olss is
inferred as 1, so I'd expect it to also be at least 1 for
vps_each_layer_is_an_ols_flag == false.
if (current->vps_each_layer_is_an_ols_flag)
vps_num_dpb_params = 0;
else
@@ -991,7 +992,7 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
else
infer(vps_sublayer_cpb_params_present_flag, 0);
ue(vps_num_ols_timing_hrd_params_minus1, 0,
- num_multi_layer_olss - 1);
+ num_multi_layer_olss > 0 ? num_multi_layer_olss - 1 : 0);
for (i = 0; i <=
current->vps_num_ols_timing_hrd_params_minus1; i++) {
uint8_t first_sublayer;
if (!current->vps_default_ptl_dpb_hrd_max_tid_flag)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
