[VDPAU] [PATCH] Fix error in sizes of H265 column width and row height, arrays

[Aaron Plattner]

On 03/13/2015 11:39 AM, José Hiram Soltren wrote:
> An NVIDIA internal hardware document noted:
>
>   #define MAX_TILE_COLS 20
>   #define MAX_TILE_ROWS 22
>
> As of this writing the VDPAU API writes:
>
>      /** Only need to set 0..num_tile_columns_minus1. The struct
>          definition reserves up to the maximum of 22. Invalid values are
>          ignored. */
>      uint16_t column_width_minus1[22];
>      /** Only need to set 0..num_tile_rows_minus1. The struct
>          definition reserves up to the maximum of 20. Invalid values are
>          ignored.*/
>      uint16_t row_height_minus1[20];
>
> This is not correct. The correct definitions ought to be:
>
>      uint16_t column_width_minus1[20];
>      uint16_t row_height_minus1[22];

Dang it.  Oh well.

> The H.265 Specification does not give an explicit range for the sizes
> of these arrays. It is possible to calculate an upper limit for a particular
> video frame implicitly using these equations:
>
> MinCbLog2SizeY = log2_min_luma_coding_block_size_minus3 + 3 (7-10)
> CtbLog2SizeY = MinCbLog2SizeY + log2_diff_max_min_luma_coding_block_size (7-11)
> CtbSizeY = 1 << CtbLog2SizeY (7-13)
> PicWidthInCtbsY = Ceil( pic_width_in_luma_samples ÷ CtbSizeY ) (7-15)
> num_tile_columns_minus1 ϵ [0, PicWidthInCtbsY − 1]
>
> (num_tile_rows_minus1 is similar)
>
> For a video with:
> log2_min_luma_coding_block_size_minus3 = 0
> log2_diff_max_min_luma_coding_block_size = 0
> pic_width_in_luma_samples = 4096
>
> num_tile_columns_minus1 < 512
>
> This seems patological. Perhaps we could cap column_width_minus1[] and
> row_height_minus1[] at 32 or 64 elements apiece if other hardware
> implementations saw a reason to do so.
>
> This change as proposed does not alter the size of VdpPictureInfoHEVC, but
> it *does* change the ABI. We can either add it as a fixup to the just
> released VDPAU 1.0, or create a follow-on patch structure. Since few have
> adopted VdpPictureInfoHEVC since Monday my preference is to fix the
> existing structure.

That's my inclination as well.  Maybe to be completely safe, it would be 
prudent to bump the VDP_DECODER_PROFILE_HEVC_* numbers as well, so that 
implementations built against libvdpau 1.1 won't report support for the 
profiles as defined in libvdpau 1.0?

> ---
>   include/vdpau/vdpau.h |    8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/vdpau/vdpau.h b/include/vdpau/vdpau.h
> index 20c6d1f..de21c00 100644
> --- a/include/vdpau/vdpau.h
> +++ b/include/vdpau/vdpau.h
> @@ -3245,13 +3245,13 @@ typedef struct {
>       /** Only valid if tiles_enabled_flag is set. Ignored otherwise. */
>       uint8_t uniform_spacing_flag;
>       /** Only need to set 0..num_tile_columns_minus1. The struct
> -        definition reserves up to the maximum of 22. Invalid values are
> +        definition reserves up to the maximum of 20. Invalid values are
>           ignored. */
> -    uint16_t column_width_minus1[22];
> +    uint16_t column_width_minus1[20];
>       /** Only need to set 0..num_tile_rows_minus1. The struct
> -        definition reserves up to the maximum of 20. Invalid values are
> +        definition reserves up to the maximum of 22. Invalid values are
>           ignored.*/
> -    uint16_t row_height_minus1[20];
> +    uint16_t row_height_minus1[22];
>       /** Only needed if tiles_enabled_flag is set. Invalid values are
>           ignored. */
>       uint8_t loop_filter_across_tiles_enabled_flag;
>


-- 
Aaron