diff mbox series

[FFmpeg-devel,03/15] avformat/fitsdec: Don't use AVBPrint for temporary storage

Message ID GV1P250MB07377A2051FBAD7B79EAD5128F302@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit b93ed5c28ecc06a0a5e54994439c9a65fa0f96c4
Headers show
Series [FFmpeg-devel,01/15] configure: Make hls demuxer select AAC, AC3 and EAC3 demuxers | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt March 23, 2024, 2:06 a.m. UTC
Most of the data in the temporary storage ends up being
returned to the user as AVPacket.data, so it makes sense
to avoid using the AVBPrint for temporary storage altogether
(in particular in light of the fact that the blocks read here
are too big for the small-string optimization anyway) and
read the data directly into AVPacket.data. This also avoids
another memcpy() from a stack buffer to the AVBPrint in ts_image()
(that could always have been avoided with av_bprint_get_buffer()).

These changes also allow to use av_append_packet(), which
greatly simplifies the code; furthermore, one can avoid cleanup
code on error as the packet is already unreferenced generically
on error.

There are two user-visible changes from this patch:
1. Truncated packets are now marked as corrupt.
2. AVPacket.pos is set (it corresponds to the discarded header
line, 80 bytes before the position corresponding to the
actual packet data).

Furthermore, this patch also removes code that triggered
a -Wtautological-constant-out-of-range-compare warning
from Clang (namely a comparison of an unsigned and INT64_MAX
in an assert).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/fitsdec.c | 80 ++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 55 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/fitsdec.c b/libavformat/fitsdec.c
index fe2dd5ad5d..6771dda327 100644
--- a/libavformat/fitsdec.c
+++ b/libavformat/fitsdec.c
@@ -24,13 +24,10 @@ 
  * FITS demuxer.
  */
 
-#include "libavutil/avassert.h"
-#include "libavutil/intreadwrite.h"
 #include "demux.h"
 #include "internal.h"
 #include "libavutil/opt.h"
 #include "libavcodec/fits.h"
-#include "libavutil/bprint.h"
 
 #define FITS_BLOCK_SIZE 2880
 
@@ -71,31 +68,31 @@  static int fits_read_header(AVFormatContext *s)
  * @param s pointer to AVFormat Context
  * @param fits pointer to FITSContext
  * @param header pointer to FITSHeader
- * @param avbuf pointer to AVBPrint to store the header
+ * @param pkt pointer to AVPacket to store the header
  * @param data_size to store the size of data part
- * @return 1 if image found, 0 if any other extension and AVERROR_INVALIDDATA otherwise
+ * @return 1 if image found, 0 if any other extension and AVERROR code otherwise
  */
-static int64_t is_image(AVFormatContext *s, FITSContext *fits, FITSHeader *header,
-                         AVBPrint *avbuf, uint64_t *data_size)
+static int is_image(AVFormatContext *s, FITSContext *fits, FITSHeader *header,
+                    AVPacket *pkt, uint64_t *data_size)
 {
     int i, ret, image = 0;
-    char buf[FITS_BLOCK_SIZE] = { 0 };
-    int64_t buf_size = 0, size = 0, t;
+    int64_t size = 0, t;
 
     do {
-        ret = avio_read(s->pb, buf, FITS_BLOCK_SIZE);
+        const uint8_t *buf, *buf_end;
+        ret = av_append_packet(s->pb, pkt, FITS_BLOCK_SIZE);
         if (ret < 0) {
             return ret;
         } else if (ret < FITS_BLOCK_SIZE) {
             return AVERROR_INVALIDDATA;
         }
 
-        av_bprint_append_data(avbuf, buf, FITS_BLOCK_SIZE);
         ret = 0;
-        buf_size = 0;
-        while(!ret && buf_size < FITS_BLOCK_SIZE) {
-            ret = avpriv_fits_header_parse_line(s, header, buf + buf_size, NULL);
-            buf_size += 80;
+        buf_end = pkt->data + pkt->size;
+        buf     = buf_end - FITS_BLOCK_SIZE;
+        while(!ret && buf < buf_end) {
+            ret = avpriv_fits_header_parse_line(s, header, buf, NULL);
+            buf += 80;
         }
     } while (!ret);
     if (ret < 0)
@@ -142,12 +139,10 @@  static int64_t is_image(AVFormatContext *s, FITSContext *fits, FITSHeader *heade
 
 static int fits_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
-    int64_t pos, ret;
     uint64_t size;
     FITSContext *fits = s->priv_data;
     FITSHeader header;
-    AVBPrint avbuf;
-    char *buf;
+    int ret;
 
     if (fits->first_image) {
         avpriv_fits_header_init(&header, STATE_SIMPLE);
@@ -155,57 +150,32 @@  static int fits_read_packet(AVFormatContext *s, AVPacket *pkt)
         avpriv_fits_header_init(&header, STATE_XTENSION);
     }
 
-    av_bprint_init(&avbuf, FITS_BLOCK_SIZE, AV_BPRINT_SIZE_UNLIMITED);
-    while ((ret = is_image(s, fits, &header, &avbuf, &size)) == 0) {
-        av_bprint_finalize(&avbuf, NULL);
-        pos = avio_skip(s->pb, size);
+    while ((ret = is_image(s, fits, &header, pkt, &size)) == 0) {
+        int64_t pos = avio_skip(s->pb, size);
         if (pos < 0)
             return pos;
 
-        av_bprint_init(&avbuf, FITS_BLOCK_SIZE, AV_BPRINT_SIZE_UNLIMITED);
         avpriv_fits_header_init(&header, STATE_XTENSION);
+        av_packet_unref(pkt);
     }
     if (ret < 0)
-        goto fail;
-
-    if (!av_bprint_is_complete(&avbuf)) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
-    }
-
-    av_assert0(avbuf.len <= INT64_MAX && size <= INT64_MAX);
-    if (avbuf.len + size > INT_MAX - 80)  {
-        ret = AVERROR_INVALIDDATA;
-        goto fail;
-    }
-    // Header is sent with the first line removed...
-    ret = av_new_packet(pkt, avbuf.len - 80 + size);
-    if (ret < 0)
-        goto fail;
+        return ret;
 
     pkt->stream_index = 0;
-    pkt->flags |= AV_PKT_FLAG_KEY;
+    pkt->flags       |= AV_PKT_FLAG_KEY;
+    pkt->duration     = 1;
+    // Header is sent with the first line removed...
+    pkt->data        += 80;
+    pkt->size        -= 80;
 
-    ret = av_bprint_finalize(&avbuf, &buf);
-    if (ret < 0) {
-        return ret;
-    }
+    if (size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE - pkt->size)
+        return AVERROR(ERANGE);
 
-    memcpy(pkt->data, buf + 80, avbuf.len - 80);
-    pkt->size = avbuf.len - 80;
-    av_freep(&buf);
-    ret = avio_read(s->pb, pkt->data + pkt->size, size);
+    ret = av_append_packet(s->pb, pkt, size);
     if (ret < 0)
         return ret;
 
-    pkt->size += ret;
-    pkt->duration = 1;
-
     return 0;
-
-fail:
-    av_bprint_finalize(&avbuf, NULL);
-    return ret;
 }
 
 static const AVOption fits_options[] = {