On 7/8/2020 4:44 PM, Michael Niedermayer wrote:
> On Tue, Jul 07, 2020 at 05:41:11PM -0300, James Almer wrote:
>> On 7/7/2020 5:07 PM, Brian Kim wrote:
>>> On Tue, Jul 7, 2020 at 6:34 AM James Almer <jamr...@gmail.com> wrote:
>>>>
>>>> If i understand this right, you could easily solve it with just the
>>>> following changes:
>>>>
>>>>> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
>>>>> index 7f9c1b632c..48a373db01 100644
>>>>> --- a/libavutil/imgutils.c
>>>>> +++ b/libavutil/imgutils.c
>>>>> @@ -126,7 +126,8 @@ int av_image_fill_pointers(uint8_t *data[4], enum
>>>>> AVPixelFormat pix_fmt, int hei
>>>>>
>>>>> if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
>>>>> desc->flags & FF_PSEUDOPAL) {
>>>>> - data[1] = ptr + size[0]; /* palette is stored here as 256 32
>>>>> bits words */
>>>>> + if (ptr)
>>>>> + data[1] = ptr + size[0]; /* palette is stored here as 256 32
>>>>> bits words */
>>>>> return size[0] + 256 * 4;
>>>>> }
>>>>>
>>>>> @@ -136,7 +137,8 @@ int av_image_fill_pointers(uint8_t *data[4], enum
>>>>> AVPixelFormat pix_fmt, int hei
>>>>> total_size = size[0];
>>>>> for (i = 1; i < 4 && has_plane[i]; i++) {
>>>>> int h, s = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
>>>>> - data[i] = data[i-1] + size[i-1];
>>>>> + if (data[i - 1])
>>>>> + data[i] = data[i - 1] + size[i - 1];
>>>>> h = (height + (1 << s) - 1) >> s;
>>>>> if (linesizes[i] > INT_MAX / h)
>>>>> return AVERROR(EINVAL);
>>>
>>> Do we have to worry about backwards compatibility here? Some places
>>> (e.g. libavcodec/decode.c:1497) have been using data to calculate the
>>> sizes.
>>
>> That code depended on undefined behavior, for both C and the
>> av_image_fill_pointers() function. So IMO no, we don't need to worry
>> about it.
>
> If you break the size = data0 - data1 usecase then
> you must bump the major abi version because if you dont then
> distros will have things break and this is our own libs that break
> because they use this before we fix it.
Leaving *data[4] zeroed when ptr == NULL would only break size = data1 -
data0 for prt == NULL. And if this is UB, then we can't even be sure the
above is guaranteed with every compiler.
In any case, I'm fine with postponing that change until after a bump. As
i said it should be a matter of adding a simple check.
The addition of av_image_fill_plane_sizes() should be enough for now.
>
> I hope iam wrong, of course because this is a mess
>
> thx
>
> [...]
>
>
> _______________________________________________
> 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".
