[x265] unsigned promotion prevents encoding frames with negative strides

[Ashok Kumar Mishra]

Hi Vittorio,

I agree that we should support negative stride and going to work on this
after sometime. Now we all are busy with some high priority tasks.
We would be very happy to see a patch from you supporting negative stride,
provided you have some spare time.

Thanks
Ashok.

On Thu, Jan 25, 2018 at 6:04 PM, Vittorio Giovara <
vittorio.giovara at gmail.com> wrote:

>
>
> On Wed, Jan 17, 2018 at 5:31 PM, chen <chenm003 at 163.com> wrote:
>
>> Hi,
>>
>> Thank you report this bug.
>> I think the root cause is not sizeof(), the negative stride is invalid in
>> encoder/decoder core.
>> To avoid these invalid input parameters, the x264 insert a middle-layer
>> that convert color space and images, but x265 doesn't it.
>>
>> Of course, crash is worst way to report invalid input parameters, we will
>> fix it soon.
>>
>> Thanks,
>> Min
>>
>> At 2018-01-18 00:24:19, "Vittorio Giovara" <vittorio.giovara at gmail.com>
>> wrote:
>>
>> Hi,
>> I'm developing an app which flips a frame in this way
>>
>>     frame->stride[0] *= -1;
>>     frame->stride[1] *= -1;
>>     frame->stride[2] *= -1;
>>     frame->data[0]    = frame->data[1] + frame->stride[0];
>>     frame->data[1]    = frame->data[2] + frame->stride[1];
>>     frame->data[2]    = frame->data[2] + (((frame->height >>
>> desc->log2_chroma_h) - 1) * -frame->stride[2]);
>>
>> and then proceeds to encode it with either x264 or x265.
>>
>> When feeding this frame to x265_encode(), I get a crash in
>> x265_upShift_16_avx2 at picyuv.cpp:322. While debugging I noticed that
>> there is something wrong in the division for the input stride: for a
>> 640x480 yuv 10 bit frame (with negative strides) the computed value is
>> 9223372036854775168 instead of -640.
>>
>> I believe the problem lies in the operation pic.stride[0] /
>> sizeof(*yShort) where the result of every division is promoted to
>> unsigned since sizeof() returns size_t (aka unsigned long).
>> Unfortunately this issue is spread across the entire codebase, affecting
>> every arithmetic operation whenever a stride is computed or updated. I
>> think this was introduced four years ago in
>> https://bitbucket.org/multicoreware/x265/commits/eadec14402d6.
>>
>> The solution would be to properly cast every sizeof() operation to
>> ssize_t or int, or modify the internal functions to operate on bytes
>> instead of pixels. The same frame fed to x264 is correctly encoded just
>> fine.
>>
>> Can anybody verify and apply a proper fix? Thank you
>> --
>> Vittorio
>>
>>
>>
> Hi, was there any progress on this?
> I agree with Derek that there is value in supporting negative strides, as
> it's a common feature in multimedia systems.
> Regards
> --
> Vittorio
>
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20180130/5ec30249/attachment.html>