On Sat, Aug 24, 2024 at 5:01 PM Frank Plowman <p...@frankplowman.com> wrote:
> 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. > oh, thank you for the explanation. Another code may assume subpic coincides with tile. it's my bad. We can merge your patch first and fix another code when we have a valid clip > > >> 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. > Thank you. Sometimes a full check isn't feasible, so we can only do our best. If it's too difficult or the cost is too high, we can leave it as is. > > 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". > _______________________________________________ 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".
