Nicolas George:
> Guo, Yejun (12021-04-01):
>>> Is this allowed to be negative? Can/should this be size_t?
>> There was a long discussion about size_t/int/uint32_t when I added
>> struct AVRegionOfInterest in frame.h, and finally size_t is not chosen.
>
> Then at least unsigned.
>
>> yes, we can add a version number (enum AVBBoxHeaderVersion, see below) at the
>> beginning of the struct, and the user needs to set it with
>> AV_BBOX_HEADER_CURRENT.
>> It is not elegant, is this what we really want?
>
> A version number does not make the structure compatible. At best,
> applications check the version and detect they do not support this one
> and report an error. At worse, they do not bother to check and it causes
> strange bugs. The version number does not help with compatibility, it
> just makes it detectable.
>
> To make the structure compatible, we have to ask ourselves: What happens
> if we update it? What breaks compatibility? Once we have an answer, we
> find a workaround.
>
> In this case, what happens is that the variable array must be at the
> end, and therefore its offset changes. And we cannot have a second
> variable array (like the name! you had to set a constant size), and we
> cannot update the type of the array elements.
>
> And the solution becomes obvious: let us store the offsets in the
> structure.
>
> So, that would be:
>
> typedef struct AVBoundingBoxHeader {
> ...
> /**
> * Offset of the array of bounding boxes.
> */
> size_t bboxes_offset;
>
> /**
> * Offset increment in the array of bounding boxes.
> */
> size_t bboxes_step;
> };
>
> Note that with that solution, we do not need the empty array, we can do
> this:
>
> AVBoundingBoxHeader *ff_bounding_box_alloc(size_t nb_bbox)
> {
> struct {
> AVBoundingBoxHeader header;
> AVBoundingBox boxes[nb_bbox];
That's a variable-length array. That's a C99 feature we do not require.
> } *ret;
> ret = av_mallocz(sizeof(*ret));
> /* add error checks */
> ret->header->bboxes_offset = (char *)&ret->boxes - (char *)ret->header;
> ret->header->bboxes_step = sizeof(*ret->boxes);
> }
>
> #define AV_BOUNDING_BOX_GET(header, idx) \
> ((AVBoundingBox *)((char *)(header) + (header)->bboxes_offset + (idx) *
> (header)->bboxes_step))
>
> You can do the same to lift the 128 limit on the name.
>
> Regards,
>
>
> _______________________________________________
> 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".
