Hi, Thanks for your review.
On 24/08/2024 04:40, Nuo Mi wrote: > Hi Frank, > thank you for the patch > On Fri, Aug 23, 2024 at 7:45 PM Frank Plowman <p...@frankplowman.com> wrote: > >> The previous logic relied on the subpicture boundaries coinciding with >> the tile boundaries. Per 6.3.1 of H.266 (V3), vertical subpicture >> boundaries are always tile boundaries however the same cannot be said >> for horizontal subpicture boundaries. > > From the spec: > "One or both of the following conditions shall be fulfilled for each > subpicture and tile: > > – All CTUs in a subpicture belong to the same tile. > > – All CTUs in a tile belong to the same subpicture." > > This suggests that the subpicture boundary coincides with a tile boundary, > right? Not always. As an example, consider a picture with only a single tile but split into two subpictures, one atop the other: Tiles Subpics Slices +-------+ +-------+ +-------+ | | | | | | | | +-------+ +-------+ | | | | | | +-------+ +-------+ +-------+ The first of the conditions is met for both subpics: all CTUs of either subpic belong to the same tile. It is clear to see, however, that the bottom boundary of the top subpicture and the top boundary of the bottom subpicture do not coincide with a tile boundary. In conjunction with the restrictions on subpic and slice layout and those on slice and tile layout, you get a guarantee that the vertical boundaries of subpics coincide with vertical boundaries of tiles, but there is no guaranteed relationship between the horizontal boundaries of subpics and tile boundaries. >> Furthermore, it is possible to >> construct an illegal bitstream where vertical subpicture boundaries are >> not coincident with tile boundaries. In these cases, the condition of >> the while loop would never be satisfied resulting in an OOB read on >> col_bd/row_bd. >> > Can we implement early checks to reject invalid streams? > If the picture boundaries are incorrect, it indicates a serious error in > the bitstream. > My first attempt at solving this problem took this approach. I found that it was not trivial to write an algorithm which ensures all the relationships set out in 6.3.1. As far as I can tell, the restrictions are only ever set out in prose in that section and never as more formal requirements on bitstream conformance. Additionally, these checks appear to be missing from VTM, so there is no reference implementation to refer to (aside: VTM is quite happy to encode and decode bitstreams with invalid partitioning layouts). As a result, I decided to take the approach of identifying where the relationships were used and making changes there instead. Even if the partitioning structure were validated elsewhere however, the change to the horizontal boundaries would still be needed as set out above. I also think the change to the vertical boundaries does no harm and serves to prevent an OOB access in the case that validation fails, however unlikely that may be. -- All the best, Frank _______________________________________________ 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".