[mrt-amd]

Fixes #5569.

Tested with reproducer in the ticket, and also verified against larger text content streams where this bug was previously being hit with some regularity.

[urvangjoshi]

This solution seems a bit fragile. Shouldn't the winner mode store the color_index_map instead, so no such logic is needed here?

[mrt-amd]

This is happening because the palette search in inter frames is currently done separately to the regular candidate processing so it doesn't get stored in the normal candidate list, hence the need to look in a different place here. An alternative approach would be to have the palette mode search store its result in the candidate list so it can be used from there like it is in intra frames. This would actually change the bitstream, though, as it might push out another candidate and refinement could then get a different answer, unlike the above which is a pure fix. Would you prefer that?

[yunqingwang1]

IIRC, in AV1, av2_rd_pick_intra_sby_mode() is only called in intra frame encoding, so this bug doesn't exist. In AV2, this function is also called in inter frame coding, and that causes this bug. Is my understanding correct? If this is the case, maybe your alternative approach is better and more fundamental? It is ok if the bitstream result changes slightly. What do you think?

[urvangjoshi]

@yunqingwang1 : maybe good if you / someone who's familiar with winner mode processing can review.
Also, curious if this has been fixed on av2-enc branch just in case.

[yunqingwang1]

@yunqingwang1 : maybe good if you / someone who's familiar with winner mode processing can review. Also, curious if this has been fixed on av2-enc branch just in case.

This seems to be inherited from the AV1. I checked av2-enc branch, this part was refactored a little, but no fix was done. Here is the code:
// Restores the saved luma palette color map.
static AVM_INLINE void restore_palette_color_map(MACROBLOCKD *xd,
BLOCK_SIZE bsize,
const MB_MODE_INFO *mbmi,
const uint8_t *color_map_src) {
if (mbmi->palette_mode_info.palette_size[PLANE_TYPE_Y] > 0) {
int block_width, block_height;
av2_get_block_dimensions(bsize, AVM_PLANE_Y, xd, &block_width,
&block_height, /rows_within_bounds/ NULL,
/cols_within_bounds/ NULL);
uint8_t *color_map_dst = xd->plane[PLANE_TYPE_Y].color_index_map;
av2_copy_array(color_map_dst, color_map_src, block_width * block_height);
}
}

@mrt-amd:
Thanks for the fix. Looks like there are 2 restore_palette_color_map (1 before winner mode processing, and 1 after). The fix handles the one after the winner mode processing. Do we need to do anything for the first one?

[mrt-amd]

@yunqingwang1 :

Looks like there are 2 restore_palette_color_map (1 before winner mode processing, and 1 after). The fix handles the one after the winner mode processing. Do we need to do anything for the first one?

No.

The first restore is loading the map from a specific list entry mode_idx with its corresponding mbmi set from that entry, so it always matches.

The second restore has two effects. If a palette mode was in the list then the map might have been overwritten by a different entry, so the restore from entry best_mode_idx with best_mbmi is needed to bring back the right map - this part is correct. However, best_mbmi might also have come from the separate palette mode search and in this case best_mode_idx as an index in the list does not correspond to the map you want. The restore therefore copies the wrong map and causes the observed corruption. The fix above is ensuring that we use the right map in this case.

[yunqingwang1]

@yunqingwang1 :

Looks like there are 2 restore_palette_color_map (1 before winner mode processing, and 1 after). The fix handles the one after the winner mode processing. Do we need to do anything for the first one?

No.

The first restore is loading the map from a specific list entry mode_idx with its corresponding mbmi set from that entry, so it always matches.

The second restore has two effects. If a palette mode was in the list then the map might have been overwritten by a different entry, so the restore from entry best_mode_idx with best_mbmi is needed to bring back the right map - this part is correct. However, best_mbmi might also have come from the separate palette mode search and in this case best_mode_idx as an index in the list does not correspond to the map you want. The restore therefore copies the wrong map and causes the observed corruption. The fix above is ensuring that we use the right map in this case.

Thanks for the explanation.