diff mbox

[FFmpeg-devel,1/2] avcodec/cbs_h265: fix valid range of num_tile_{columns, rows}_minus1 in H265RawPPS

Message ID 20190620174525.6515-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer June 20, 2019, 5:45 p.m. UTC
The spec states they can't be higher than the respective dimensions of the
stream in CTBs.

Signed-off-by: James Almer <jamrial@gmail.com>
---
I don't think it's wise further limiting the range to the maximum currently
defined for level 6.2 using those two HEVC_ defines, since a stream could in
theory go beyond them (New levels defined in the future?), but the alternative
is making the column_width_minus1 and row_height_minus1 arrays much bigger, or
dinamically allocated.

 libavcodec/cbs_h265_syntax_template.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Gyan Doshi June 21, 2019, 4:24 a.m. UTC | #1
On 20-06-2019 11:15 PM, James Almer wrote:
> The spec states they can't be higher than the respective dimensions of the
> stream in CTBs.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> I don't think it's wise further limiting the range to the maximum currently
> defined for level 6.2 using those two HEVC_ defines, since a stream could in
> theory go beyond them (New levels defined in the future?), but the alternative
> is making the column_width_minus1 and row_height_minus1 arrays much bigger, or
> dinamically allocated.
dinamically--> dynamically

>
>   libavcodec/cbs_h265_syntax_template.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index f279d283d9..d2a20ddb35 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1045,8 +1045,17 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw,
>       flag(entropy_coding_sync_enabled_flag);
>   
>       if (current->tiles_enabled_flag) {
> -        ue(num_tile_columns_minus1, 0, HEVC_MAX_TILE_COLUMNS);
> -        ue(num_tile_rows_minus1,    0, HEVC_MAX_TILE_ROWS);
> +        unsigned int min_cb_log2_size_y = sps->log2_min_luma_coding_block_size_minus3 + 3;
> +        unsigned int ctb_log2_size_y =
> +            min_cb_log2_size_y + sps->log2_diff_max_min_luma_coding_block_size;
> +        unsigned int ctb_size_y = 1 << ctb_log2_size_y;
> +        unsigned int pic_width_in_ctbs_y =
> +            (sps->pic_width_in_luma_samples + ctb_size_y - 1) / ctb_size_y;
> +        unsigned int pic_height_in_ctbs_y =
> +            (sps->pic_height_in_luma_samples + ctb_size_y - 1) / ctb_size_y;
> +
> +        ue(num_tile_columns_minus1, 0, FFMIN(pic_width_in_ctbs_y - 1, HEVC_MAX_TILE_COLUMNS - 1));
> +        ue(num_tile_rows_minus1,    0, FFMIN(pic_height_in_ctbs_y - 1, HEVC_MAX_TILE_ROWS - 1));
>           flag(uniform_spacing_flag);
>           if (!current->uniform_spacing_flag) {
>               for (i = 0; i < current->num_tile_columns_minus1; i++)

Gyan
James Almer June 21, 2019, 12:19 p.m. UTC | #2
On 6/21/2019 1:24 AM, Gyan wrote:
> 
> 
> On 20-06-2019 11:15 PM, James Almer wrote:
>> The spec states they can't be higher than the respective dimensions of
>> the
>> stream in CTBs.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> I don't think it's wise further limiting the range to the maximum
>> currently
>> defined for level 6.2 using those two HEVC_ defines, since a stream
>> could in
>> theory go beyond them (New levels defined in the future?), but the
>> alternative
>> is making the column_width_minus1 and row_height_minus1 arrays much
>> bigger, or
>> dinamically allocated.
> dinamically--> dynamically

Thanks, but that's not part of the commit message, so it'll not make it
out of this mailing list.

> 
>>
>>   libavcodec/cbs_h265_syntax_template.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h265_syntax_template.c
>> b/libavcodec/cbs_h265_syntax_template.c
>> index f279d283d9..d2a20ddb35 100644
>> --- a/libavcodec/cbs_h265_syntax_template.c
>> +++ b/libavcodec/cbs_h265_syntax_template.c
>> @@ -1045,8 +1045,17 @@ static int FUNC(pps)(CodedBitstreamContext
>> *ctx, RWContext *rw,
>>       flag(entropy_coding_sync_enabled_flag);
>>         if (current->tiles_enabled_flag) {
>> -        ue(num_tile_columns_minus1, 0, HEVC_MAX_TILE_COLUMNS);
>> -        ue(num_tile_rows_minus1,    0, HEVC_MAX_TILE_ROWS);
>> +        unsigned int min_cb_log2_size_y =
>> sps->log2_min_luma_coding_block_size_minus3 + 3;
>> +        unsigned int ctb_log2_size_y =
>> +            min_cb_log2_size_y +
>> sps->log2_diff_max_min_luma_coding_block_size;
>> +        unsigned int ctb_size_y = 1 << ctb_log2_size_y;
>> +        unsigned int pic_width_in_ctbs_y =
>> +            (sps->pic_width_in_luma_samples + ctb_size_y - 1) /
>> ctb_size_y;
>> +        unsigned int pic_height_in_ctbs_y =
>> +            (sps->pic_height_in_luma_samples + ctb_size_y - 1) /
>> ctb_size_y;
>> +
>> +        ue(num_tile_columns_minus1, 0, FFMIN(pic_width_in_ctbs_y - 1,
>> HEVC_MAX_TILE_COLUMNS - 1));
>> +        ue(num_tile_rows_minus1,    0, FFMIN(pic_height_in_ctbs_y -
>> 1, HEVC_MAX_TILE_ROWS - 1));
>>           flag(uniform_spacing_flag);
>>           if (!current->uniform_spacing_flag) {
>>               for (i = 0; i < current->num_tile_columns_minus1; i++)
> 
> Gyan
> _______________________________________________
> 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".
diff mbox

Patch

diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
index f279d283d9..d2a20ddb35 100644
--- a/libavcodec/cbs_h265_syntax_template.c
+++ b/libavcodec/cbs_h265_syntax_template.c
@@ -1045,8 +1045,17 @@  static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw,
     flag(entropy_coding_sync_enabled_flag);
 
     if (current->tiles_enabled_flag) {
-        ue(num_tile_columns_minus1, 0, HEVC_MAX_TILE_COLUMNS);
-        ue(num_tile_rows_minus1,    0, HEVC_MAX_TILE_ROWS);
+        unsigned int min_cb_log2_size_y = sps->log2_min_luma_coding_block_size_minus3 + 3;
+        unsigned int ctb_log2_size_y =
+            min_cb_log2_size_y + sps->log2_diff_max_min_luma_coding_block_size;
+        unsigned int ctb_size_y = 1 << ctb_log2_size_y;
+        unsigned int pic_width_in_ctbs_y =
+            (sps->pic_width_in_luma_samples + ctb_size_y - 1) / ctb_size_y;
+        unsigned int pic_height_in_ctbs_y =
+            (sps->pic_height_in_luma_samples + ctb_size_y - 1) / ctb_size_y;
+
+        ue(num_tile_columns_minus1, 0, FFMIN(pic_width_in_ctbs_y - 1, HEVC_MAX_TILE_COLUMNS - 1));
+        ue(num_tile_rows_minus1,    0, FFMIN(pic_height_in_ctbs_y - 1, HEVC_MAX_TILE_ROWS - 1));
         flag(uniform_spacing_flag);
         if (!current->uniform_spacing_flag) {
             for (i = 0; i < current->num_tile_columns_minus1; i++)