Hi Daniel,
Question about tile info structure.
struct v4l2_av1_tile_info {
__u8 flags;
__u32 mi_col_starts[V4L2_AV1_MAX_TILE_COLS + 1];
__u32 mi_row_starts[V4L2_AV1_MAX_TILE_ROWS + 1];
__u32 width_in_sbs_minus_1[V4L2_AV1_MAX_TILE_COLS];
__u32 height_in_sbs_minus_1[V4L2_AV1_MAX_TILE_ROWS];
I see below from the spec and gstreamer implementation
for width_in_sbs_minus_1 and height_in_sbs_minus_1 computation.
sb_cols = seq_header->use_128x128_superblock ?
((parser->state.mi_cols + 31) >> 5) : ((parser->state.mi_cols + 15) >> 4);
sb_rows = seq_header->use_128x128_superblock ? ((parser->state.mi_rows +
31) >> 5) : ((parser->state.mi_rows + 15) >> 4);
Are we confident that V4L2_AV1_MAX_TILE_COLS is good enough size for
width_in_sbs_minus_1?
Or does it potentially need to be V4L2_AV1_MAX_TILE_COLS+1?
I am asking to double check because I see V4L2_AV1_MAX_TILE_COLS+1
used for corresponding field in libgav1.
int tile_column_width_in_superblocks[kMaxTileColumns + 1];
https://source.chromium.org/chromium/chromium/src/+/main:third_party/libgav1/src/src/utils/types.h;l=291
Steve
On Wed, May 11, 2022 at 1:59 PM Steve Cho <stevecho@xxxxxxxxxxxx> wrote:
>
> Hi Daniel,
>
> I think the below definition is expected to cause a build error.
>
> +struct v4l2_av1_loop_restoration {
> + u8 flags;
>
> s/u8/__u8/ is needed.
>
> At least, this change was needed to fix this build error on Chromium
> build environment.
>
> Steve
>
> On Tue, May 10, 2022 at 9:30 AM Daniel Almeida
> <daniel.almeida@xxxxxxxxxxxxx> wrote:
> >
> > Hi Steve,
> >
> > > Hi Daniel,
> > >
> > > Found a minor typo.
> > >
> > >> See enum_v4l2_av1_frame_restoration_type.
> > > Assume you meant v4l2_av1_frame_restoration_type instead here.
> >
> >
> > Thanks for the heads up, this will be fixed in v3.
> >
> > -- Daniel
> >
