[FFmpeg-devel] [PATCH 0/4] Get bits buffer end

[FFmpeg-devel] [PATCH 0/4] Get bits buffer end

[ffmpegagent]

This patchset eliminates GetBitContext.buffer_end (which is mostly unused).
It therefore automatically fixes the UB that happens upon init_get_bits()
failure (when using pre-C23).

Andreas Rheinhardt (4):
  avcodec/bytestream: Add const where appropriate
  avcodec/vvc/dec: Don't use GetBit-API when byte-aligned
  avcodec/get_bits: Add get_bits_bytesize()
  avcodec/get_bits: Remove GetBitContext.buffer_end

 libavcodec/bitstream.h          |  2 ++
 libavcodec/bitstream_template.h |  8 ++++++++
 libavcodec/bytestream.h         | 16 ++++++++--------
 libavcodec/get_bits.h           | 21 ++++++++++++++++-----
 libavcodec/h263dec.c            |  7 ++++---
 libavcodec/h264_ps.c            |  4 ++--
 libavcodec/hevc/hevcdec.c       |  2 +-
 libavcodec/hevc/ps.c            |  6 +++---
 libavcodec/mpeg12dec.c          |  5 +++--
 libavcodec/vvc/dec.c            | 29 ++++++++++++++++-------------
 10 files changed, 63 insertions(+), 37 deletions(-)


base-commit: 0fe9f25e76163613505f77a8036dc62524070f0a
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-106%2Fmkver%2FGetBits_buffer_end-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-106/mkver/GetBits_buffer_end-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/106
-- 
ffmpeg-codebot
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/4] avcodec/bytestream: Add const where appropriate

[Andreas Rheinhardt]

From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/bytestream.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h
index 67080604b9..9c13a2791b 100644
--- a/libavcodec/bytestream.h
+++ b/libavcodec/bytestream.h
@@ -77,11 +77,11 @@ static av_always_inline type bytestream2_get_ ## name(GetByteContext *g)       \
     }                                                                          \
     return bytestream2_get_ ## name ## u(g);                                   \
 }                                                                              \
-static av_always_inline type bytestream2_peek_ ## name ## u(GetByteContext *g) \
+static av_always_inline type bytestream2_peek_ ## name ## u(const GetByteContext *g) \
 {                                                                              \
     return read(g->buffer);                                                    \
 }                                                                              \
-static av_always_inline type bytestream2_peek_ ## name(GetByteContext *g)      \
+static av_always_inline type bytestream2_peek_ ## name(const GetByteContext *g)\
 {                                                                              \
     if (g->buffer_end - g->buffer < bytes)                                     \
         return 0;                                                              \
@@ -155,12 +155,12 @@ static av_always_inline void bytestream2_init_writer(PutByteContext *p,
     p->eof          = 0;
 }
 
-static av_always_inline int bytestream2_get_bytes_left(GetByteContext *g)
+static av_always_inline int bytestream2_get_bytes_left(const GetByteContext *g)
 {
     return g->buffer_end - g->buffer;
 }
 
-static av_always_inline int bytestream2_get_bytes_left_p(PutByteContext *p)
+static av_always_inline int bytestream2_get_bytes_left_p(const PutByteContext *p)
 {
     return p->buffer_end - p->buffer;
 }
@@ -189,22 +189,22 @@ static av_always_inline void bytestream2_skip_p(PutByteContext *p,
     p->buffer += size2;
 }
 
-static av_always_inline int bytestream2_tell(GetByteContext *g)
+static av_always_inline int bytestream2_tell(const GetByteContext *g)
 {
     return (int)(g->buffer - g->buffer_start);
 }
 
-static av_always_inline int bytestream2_tell_p(PutByteContext *p)
+static av_always_inline int bytestream2_tell_p(const PutByteContext *p)
 {
     return (int)(p->buffer - p->buffer_start);
 }
 
-static av_always_inline int bytestream2_size(GetByteContext *g)
+static av_always_inline int bytestream2_size(const GetByteContext *g)
 {
     return (int)(g->buffer_end - g->buffer_start);
 }
 
-static av_always_inline int bytestream2_size_p(PutByteContext *p)
+static av_always_inline int bytestream2_size_p(const PutByteContext *p)
 {
     return (int)(p->buffer_end - p->buffer_start);
 }
-- 
ffmpeg-codebot

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/4] avcodec/vvc/dec: Don't use GetBit-API when byte-aligned

[Andreas Rheinhardt]

From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vvc/dec.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index 7930d64a05..90fff3a03f 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -20,6 +20,8 @@
  * License along with FFmpeg; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
+
+#include "libavcodec/bytestream.h"
 #include "libavcodec/codec_internal.h"
 #include "libavcodec/decode.h"
 #include "libavcodec/hwaccel_internal.h"
@@ -509,13 +511,14 @@ static int slices_realloc(VVCFrameContext *fc)
     return 0;
 }
 
-static int get_ep_size(const H266RawSliceHeader *rsh, GetBitContext *gb, const H2645NAL *nal, const int header_size, const int ep_index)
+static int get_ep_size(const H266RawSliceHeader *rsh, const GetByteContext *gb,
+                       const H2645NAL *nal, const int header_size, const int ep_index)
 {
     int size;
 
     if (ep_index < rsh->num_entry_points) {
         int skipped = 0;
-        int64_t start =  (gb->index >> 3);
+        int64_t start = bytestream2_tell(gb);
         int64_t end = start + rsh->sh_entry_point_offset_minus1[ep_index] + 1;
         while (skipped < nal->skipped_bytes && nal->skipped_bytes_pos[skipped] <= start + header_size) {
             skipped++;
@@ -525,26 +528,27 @@ static int get_ep_size(const H266RawSliceHeader *rsh, GetBitContext *gb, const H
             skipped++;
         }
         size = end - start;
-        size = av_clip(size, 0, get_bits_left(gb) / 8);
+        size = av_clip(size, 0, bytestream2_get_bytes_left(gb));
     } else {
-        size = get_bits_left(gb) / 8;
+        size = bytestream2_get_bytes_left(gb);
     }
     return size;
 }
 
-static int ep_init_cabac_decoder(EntryPoint *ep, GetBitContext *gb, const int size)
+static int ep_init_cabac_decoder(EntryPoint *ep, GetByteContext *gb, const int size)
 {
     int ret;
 
-    av_assert0(gb->buffer + get_bits_count(gb) / 8 + size <= gb->buffer_end);
-    ret = ff_init_cabac_decoder (&ep->cc, gb->buffer + get_bits_count(gb) / 8, size);
+    av_assert0(size <= bytestream2_get_bytes_left(gb));
+    ret = ff_init_cabac_decoder(&ep->cc, gb->buffer, size);
     if (ret < 0)
         return ret;
-    skip_bits(gb, size * 8);
+    bytestream2_skipu(gb, size);
     return 0;
 }
 
-static int ep_init(EntryPoint *ep, const int ctu_addr, const int ctu_end, GetBitContext *gb, const int size)
+static int ep_init(EntryPoint *ep, const int ctu_addr, const int ctu_end,
+                   GetByteContext *gb, const int size)
 {
     const int ret = ep_init_cabac_decoder(ep, gb, size);
 
@@ -567,7 +571,7 @@ static int slice_init_entry_points(SliceContext *sc,
     const H266RawSlice *slice = unit->content_ref;
     int nb_eps                = sh->r->num_entry_points + 1;
     int ctu_addr              = 0;
-    GetBitContext gb;
+    GetByteContext gb;
     int ret;
 
     if (sc->nb_eps != nb_eps) {
@@ -578,9 +582,8 @@ static int slice_init_entry_points(SliceContext *sc,
         sc->nb_eps = nb_eps;
     }
 
-    ret = init_get_bits8(&gb, slice->data, slice->data_size);
-    if (ret < 0)
-        return ret;
+    bytestream2_init(&gb, slice->data, slice->data_size);
+
     for (int i = 0; i < sc->nb_eps; i++)
     {
         const int size    = get_ep_size(sc->sh.r, &gb, nal, slice->header_size, i);
-- 
ffmpeg-codebot

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/4] avcodec/get_bits: Add get_bits_bytesize()

[Andreas Rheinhardt]

From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>

And use it to avoid accesses to GetBitContext.buffer_end.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/bitstream.h          |  2 ++
 libavcodec/bitstream_template.h |  8 ++++++++
 libavcodec/get_bits.h           | 15 +++++++++++++++
 libavcodec/h263dec.c            |  7 ++++---
 libavcodec/h264_ps.c            |  4 ++--
 libavcodec/hevc/hevcdec.c       |  2 +-
 libavcodec/hevc/ps.c            |  6 +++---
 libavcodec/mpeg12dec.c          |  5 +++--
 8 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/libavcodec/bitstream.h b/libavcodec/bitstream.h
index 35b7873b9c..e4d96af710 100644
--- a/libavcodec/bitstream.h
+++ b/libavcodec/bitstream.h
@@ -82,6 +82,7 @@
 # define bits_init8         bits_init8_le
 # define bits_tell          bits_tell_le
 # define bits_size          bits_size_le
+# define bits_bytesize      bits_bytesize_le
 # define bits_left          bits_left_le
 # define bits_read_bit      bits_read_bit_le
 # define bits_read_nz       bits_read_nz_le
@@ -111,6 +112,7 @@
 # define bits_init8         bits_init8_be
 # define bits_tell          bits_tell_be
 # define bits_size          bits_size_be
+# define bits_bytesize      bits_bytesize_be
 # define bits_left          bits_left_be
 # define bits_read_bit      bits_read_bit_be
 # define bits_read_nz       bits_read_nz_be
diff --git a/libavcodec/bitstream_template.h b/libavcodec/bitstream_template.h
index bbb8dfa555..773d40ef14 100644
--- a/libavcodec/bitstream_template.h
+++ b/libavcodec/bitstream_template.h
@@ -156,6 +156,14 @@ static inline int BS_FUNC(size)(const BSCTX *bc)
     return bc->size_in_bits;
 }
 
+/**
+ * Return buffer size in bytes.
+ */
+static inline int BS_FUNC(bytesize)(const BSCTX *bc, int round_up)
+{
+    return (bc->size_in_bits + (round_up ? 7 : 0)) >> 3;
+}
+
 /**
  * Return the number of the bits left in a buffer.
  */
diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index 1954296569..c64540cf95 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -76,6 +76,7 @@
 typedef BitstreamContext GetBitContext;
 
 #define get_bits_count      bits_tell
+#define get_bits_bytesize   bits_bytesize
 #define get_bits_left       bits_left
 #define skip_bits_long      bits_skip
 #define skip_bits           bits_skip
@@ -251,6 +252,20 @@ static inline int get_bits_count(const GetBitContext *s)
     return s->index;
 }
 
+/**
+ * Get the size of the GetBitContext's buffer in bytes.
+ *
+ * @param s        the GetBitContext
+ * @param round_up If set, the number of bits will be rounded up to full bytes;
+ *                 this does not matter if the number of bits is known to be
+ *                 a multiple of eight, e.g. if the GetBitContext has been
+ *                 initialized with init_get_bits8.
+ */
+static inline int get_bits_bytesize(const GetBitContext *s, int round_up)
+{
+    return (s->size_in_bits + (round_up ? 7 : 0)) >> 3;
+}
+
 /**
  * Skips the specified number of bits.
  * @param n the number of bits to skip,
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index b2d0f9c409..3821472e91 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -208,7 +208,8 @@ static int decode_slice(H263DecContext *const h)
 
     if (h->c.avctx->hwaccel) {
         const uint8_t *start = h->gb.buffer + get_bits_count(&h->gb) / 8;
-        ret = FF_HW_CALL(h->c.avctx, decode_slice, start, h->gb.buffer_end - start);
+        ret = FF_HW_CALL(h->c.avctx, decode_slice, start,
+                         get_bits_bytesize(&h->gb, 0) - get_bits_count(&h->gb) / 8);
         // ensure we exit decode loop
         h->c.mb_y = h->c.mb_height;
         return ret;
@@ -372,7 +373,7 @@ static int decode_slice(H263DecContext *const h)
     if (h->c.codec_id == AV_CODEC_ID_H263          &&
         (h->c.workaround_bugs & FF_BUG_AUTODETECT) &&
         get_bits_left(&h->gb) >= 64                &&
-        AV_RB64(h->gb.buffer_end - 8) == 0xCDCDCDCDFC7F0000) {
+        AV_RB64(h->gb.buffer + (get_bits_bytesize(&h->gb, 0) - 8)) == 0xCDCDCDCDFC7F0000) {
 
         h->padding_bug_score += 32;
     }
@@ -546,7 +547,7 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict,
 
     if (avctx->hwaccel) {
         ret = FF_HW_CALL(avctx, start_frame, NULL,
-                         h->gb.buffer, h->gb.buffer_end - h->gb.buffer);
+                         h->gb.buffer, get_bits_bytesize(&h->gb, 0));
         if (ret < 0 )
             return ret;
     }
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index c698f1b80d..3a3cad7de7 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -294,7 +294,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
     if (!sps)
         return AVERROR(ENOMEM);
 
-    sps->data_size = gb->buffer_end - gb->buffer;
+    sps->data_size = get_bits_bytesize(gb, 1);
     if (sps->data_size > sizeof(sps->data)) {
         av_log(avctx, AV_LOG_DEBUG, "Truncating likely oversized SPS\n");
         sps->data_size = sizeof(sps->data);
@@ -712,7 +712,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
     if (!pps)
         return AVERROR(ENOMEM);
 
-    pps->data_size = gb->buffer_end - gb->buffer;
+    pps->data_size = get_bits_bytesize(gb, 1);
     if (pps->data_size > sizeof(pps->data)) {
         av_log(avctx, AV_LOG_DEBUG, "Truncating likely oversized PPS "
                "(%"SIZE_SPECIFIER" > %"SIZE_SPECIFIER")\n",
diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 797c9c76c9..21ecf063c5 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -2752,7 +2752,7 @@ static int hls_decode_entry(HEVCContext *s, GetBitContext *gb)
     const HEVCPPS   *const pps = s->pps;
     const HEVCSPS   *const sps = pps->sps;
     const uint8_t *slice_data = gb->buffer + s->sh.data_offset;
-    const size_t   slice_size = gb->buffer_end - gb->buffer - s->sh.data_offset;
+    const size_t   slice_size = get_bits_bytesize(gb, 1) - s->sh.data_offset;
     int ctb_size    = 1 << sps->log2_ctb_size;
     int more_data   = 1;
     int x_ctb       = 0;
diff --git a/libavcodec/hevc/ps.c b/libavcodec/hevc/ps.c
index 4b021ea9c1..57125d59c1 100644
--- a/libavcodec/hevc/ps.c
+++ b/libavcodec/hevc/ps.c
@@ -763,7 +763,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx,
 {
     int i;
     int vps_id = get_bits(gb, 4);
-    ptrdiff_t nal_size = gb->buffer_end - gb->buffer;
+    ptrdiff_t nal_size = get_bits_bytesize(gb, 1);
     int ret = AVERROR_INVALIDDATA;
     uint64_t layer1_id_included = 0;
     unsigned vps_base_layer_internal_flag, vps_base_layer_available_flag;
@@ -1710,7 +1710,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx,
 
     av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n");
 
-    sps->data_size = gb->buffer_end - gb->buffer;
+    sps->data_size = get_bits_bytesize(gb, 1);
     sps->data = av_memdup(gb->buffer, sps->data_size);
     if (!sps->data) {
         ret = AVERROR(ENOMEM);
@@ -2165,7 +2165,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx,
     const HEVCSPS *sps = NULL;
     const HEVCVPS *vps = NULL;
     int i, ret = 0;
-    ptrdiff_t nal_size = gb->buffer_end - gb->buffer;
+    ptrdiff_t nal_size = get_bits_bytesize(gb, 1);
     unsigned int pps_id = get_ue_golomb_long(gb);
     unsigned log2_parallel_merge_level_minus2;
     HEVCPPS *pps;
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 998f6aa2e5..3ea8d02e1b 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1635,6 +1635,7 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
 {
     Mpeg12SliceContext *const s = *(void **) arg;
     const uint8_t *buf  = s->gb.buffer;
+    const uint8_t *end  = buf + get_bits_bytesize(&s->gb, 0);
     int mb_y            = s->c.start_mb_y;
     const int field_pic = s->c.picture_structure != PICT_FRAME;
 
@@ -1644,7 +1645,7 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
         uint32_t start_code;
         int ret;
 
-        ret = mpeg_decode_slice(s, mb_y, &buf, s->gb.buffer_end - buf);
+        ret = mpeg_decode_slice(s, mb_y, &buf, end - buf);
         emms_c();
         ff_dlog(c, "ret:%d resync:%d/%d mb:%d/%d ts:%d/%d ec:%d\n",
                 ret, s->c.resync_mb_x, s->c.resync_mb_y, s->c.mb_x, s->c.mb_y,
@@ -1666,7 +1667,7 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
             return 0;
 
         start_code = -1;
-        buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code);
+        buf        = avpriv_find_start_code(buf, end, &start_code);
         if (start_code < SLICE_MIN_START_CODE || start_code > SLICE_MAX_START_CODE)
             return AVERROR_INVALIDDATA;
         mb_y       = start_code - SLICE_MIN_START_CODE;
-- 
ffmpeg-codebot

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/4] avcodec/get_bits: Remove GetBitContext.buffer_end

[Andreas Rheinhardt]

From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>

It is unused. Furthermore, this automatically fixes the issue
that init_get_bits() failure would lead to NULL + 0 (when
setting buffer_end) which is UB before C23. This happened
in the fic-avi and fic-avi-skip_cursor FATE-tests.

This saved 7296B of .text here.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/get_bits.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index c64540cf95..80a77f0ae3 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -107,7 +107,7 @@ typedef BitstreamContext GetBitContext;
 #else   // CACHED_BITSTREAM_READER
 
 typedef struct GetBitContext {
-    const uint8_t *buffer, *buffer_end;
+    const uint8_t *buffer;
     int index;
     int size_in_bits;
     int size_in_bits_plus8;
@@ -512,7 +512,6 @@ static inline unsigned int show_bits_long(GetBitContext *s, int n)
 static inline int init_get_bits(GetBitContext *s, const uint8_t *buffer,
                                 int bit_size)
 {
-    int buffer_size;
     int ret = 0;
 
     if (bit_size >= INT_MAX - FFMAX(7, AV_INPUT_BUFFER_PADDING_SIZE*8) || bit_size < 0 || !buffer) {
@@ -521,12 +520,9 @@ static inline int init_get_bits(GetBitContext *s, const uint8_t *buffer,
         ret         = AVERROR_INVALIDDATA;
     }
 
-    buffer_size = (bit_size + 7) >> 3;
-
     s->buffer             = buffer;
     s->size_in_bits       = bit_size;
     s->size_in_bits_plus8 = bit_size + 8;
-    s->buffer_end         = buffer + buffer_size;
     s->index              = 0;
 
     return ret;
-- 
ffmpeg-codebot
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".