avcodec/qtrleenc: Fix negative linesizes, don't use NULL + offset

Before commit f1e17eb446577180ee9976730aacb46563766518, the qtrle
encoder had undefined pointer arithmetic: Outside of a loop, two
pointers were set to point to the ith element (with index i-1) of
a line of a frame. At the end of each loop iteration, these pointers
were decremented, so that they pointed to the -1th element of the line
after the loop. Furthermore, one of these pointers can be NULL (in which
case all pointer arithmetic is automatically undefined behaviour).

Commit f1e17eb44 added a check in order to ensure that the elements
never point to the -1th element of the array: The pointers are only
decremented if they are bigger than the frame's base pointer
(i.e. AVFrame.data[0]). Yet this check does not work at all in case of
negative linesizes; furthermore in case the pointer that can be NULL is
NULL initializing it still involves undefined pointer arithmetic.

This commit fixes both of these issues: First, non-NULL pointers are
initialized to point to the element after the ith element and
decrementing is moved to the beginning of the loop. Second, if a pointer
is NULL, it is just made to point to the other pointer, as this allows
to avoid checks before decrementing it.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>

1 files changed, 11 insertions, 8 deletions

diff --git a/libavcodec/qtrleenc.c b/libavcodec/qtrleenc.c
index 0f5bedcdcd..f7eee4a591 100644
--- a/libavcodec/qtrleenc.c
+++ b/libavcodec/qtrleenc.c
@@ -155,10 +155,14 @@ static void qtrle_encode_line(QtrleEncContext *s, const AVFrame *p, int line, ui
     int sec_lowest_bulk_cost;
     int sec_lowest_bulk_cost_index;
 
-    uint8_t *this_line = p->               data[0] + line*p->               linesize[0] +
-        (width - 1)*s->pixel_size;
-    uint8_t *prev_line = s->previous_frame->data[0] + line * s->previous_frame->linesize[0] +
-        (width - 1)*s->pixel_size;
+    const uint8_t *this_line = p->data[0] + line * p->linesize[0] + width * s->pixel_size;
+    /* There might be no earlier frame if the current frame is a keyframe.
+     * So just use a pointer to the current frame to avoid a check
+     * to avoid NULL - s->pixel_size (which is undefined behaviour). */
+    const uint8_t *prev_line = s->key_frame ? this_line
+                                            : s->previous_frame->data[0]
+                                              + line * s->previous_frame->linesize[0]
+                                              + width * s->pixel_size;
 
     s->length_table[width] = 0;
     skipcount = 0;
@@ -175,6 +179,9 @@ static void qtrle_encode_line(QtrleEncContext *s, const AVFrame *p, int line, ui
 
         int prev_bulk_cost;
 
+        this_line -= s->pixel_size;
+        prev_line -= s->pixel_size;
+
         /* If our lowest bulk cost index is too far away, replace it
          * with the next lowest bulk cost */
         if (FFMIN(width, i + MAX_RLE_BULK) < lowest_bulk_cost_index) {
@@ -259,10 +266,6 @@ static void qtrle_encode_line(QtrleEncContext *s, const AVFrame *p, int line, ui
         /* These bulk costs increase every iteration */
         lowest_bulk_cost += s->pixel_size;
         sec_lowest_bulk_cost += s->pixel_size;
-        if (this_line >= p->data[0] + s->pixel_size)
-            this_line -= s->pixel_size;
-        if (prev_line >= s->previous_frame->data[0] + s->pixel_size)
-            prev_line -= s->pixel_size;
     }
 
     /* Good! Now we have the best sequence for this line, let's output it. */