diff mbox series

[FFmpeg-devel] avformat/westwood_vqa: Store VQFL codebook chunks

Message ID 20210921182502.7807-2-pekka.vaananen@iki.fi
State New
Headers show
Series [FFmpeg-devel] avformat/westwood_vqa: Store VQFL codebook chunks
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Pekka Väänänen Sept. 21, 2021, 6:25 p.m. UTC
High color 15-bit VQA3 video streams contain high level chunks with
only codebook updates that shouldn't be considered new frames. Now
the demuxer stores a reference to such VQFL chunks and returns them
later along with a VQFR chunk with full frame data.
---
 libavformat/westwood_vqa.c | 51 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

Comments

Andreas Rheinhardt Sept. 21, 2021, 6:46 p.m. UTC | #1
Pekka Väänänen:
> High color 15-bit VQA3 video streams contain high level chunks with
> only codebook updates that shouldn't be considered new frames. Now
> the demuxer stores a reference to such VQFL chunks and returns them
> later along with a VQFR chunk with full frame data.
> ---
>  libavformat/westwood_vqa.c | 51 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/westwood_vqa.c b/libavformat/westwood_vqa.c
> index 77df007c11..cc859c58ca 100644
> --- a/libavformat/westwood_vqa.c
> +++ b/libavformat/westwood_vqa.c
> @@ -1,6 +1,7 @@
>  /*
>   * Westwood Studios VQA Format Demuxer
> - * Copyright (c) 2003 The FFmpeg project
> + * Copyright (c) 2003 Mike Melanson <melanson@pcisys.net>
> + * Copyright (c) 2021 Pekka Väänänen <pekka.vaananen@iki.fi>
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -30,6 +31,7 @@
>  
>  #include "libavutil/intreadwrite.h"
>  #include "avformat.h"
> +#include "avio_internal.h"
>  #include "internal.h"
>  
>  #define FORM_TAG MKBETAG('F', 'O', 'R', 'M')
> @@ -40,6 +42,7 @@
>  #define SND1_TAG MKBETAG('S', 'N', 'D', '1')
>  #define SND2_TAG MKBETAG('S', 'N', 'D', '2')
>  #define VQFR_TAG MKBETAG('V', 'Q', 'F', 'R')
> +#define VQFL_TAG MKBETAG('V', 'Q', 'F', 'L')
>  
>  /* don't know what these tags are for, but acknowledge their existence */
>  #define CINF_TAG MKBETAG('C', 'I', 'N', 'F')
> @@ -60,6 +63,8 @@ typedef struct WsVqaDemuxContext {
>      int sample_rate;
>      int audio_stream_index;
>      int video_stream_index;
> +    int64_t vqfl_chunk_pos;
> +    int vqfl_chunk_size;
>  } WsVqaDemuxContext;
>  
>  static int wsvqa_probe(const AVProbeData *p)
> @@ -120,6 +125,8 @@ static int wsvqa_read_header(AVFormatContext *s)
>      wsvqa->channels     = header[26];
>      wsvqa->bps          = header[27];
>      wsvqa->audio_stream_index = -1;
> +    wsvqa->vqfl_chunk_pos     = 0;
> +    wsvqa->vqfl_chunk_size    = 0;
>  
>      s->ctx_flags |= AVFMTCTX_NOHEADER;
>  
> @@ -172,7 +179,21 @@ static int wsvqa_read_packet(AVFormatContext *s,
>  
>          skip_byte = chunk_size & 0x01;
>  
> -        if ((chunk_type == SND0_TAG) || (chunk_type == SND1_TAG) ||
> +        if (chunk_type == VQFL_TAG) {
> +            /* Each VQFL chunk carries only a codebook update inside which must be applied
> +             * before the next VQFR is rendered. That's why we stash the VQFL offset here
> +             * so it can be combined with the next VQFR packet. This way each packet
> +             * includes a whole frame as expected. */
> +            wsvqa->vqfl_chunk_pos = avio_tell(pb);
> +            wsvqa->vqfl_chunk_size = (int)(chunk_size);
> +            if (wsvqa->vqfl_chunk_size < 0 || wsvqa->vqfl_chunk_size > 3 * (1 << 20))
> +                return AVERROR_INVALIDDATA;
> +            /* We need a big seekback buffer because there can be SNxx, VIEW and ZBUF
> +             * chunks in the stream before we reach VQFR and seek back here. */

This code also reads the VQFR chunk before seeking back, so this needs
to be changed or accounted for.

> +            ffio_ensure_seekback(pb, wsvqa->vqfl_chunk_size + 512 * 1024);
> +            avio_skip(pb, chunk_size + skip_byte);
> +            continue;
> +        } else if ((chunk_type == SND0_TAG) || (chunk_type == SND1_TAG) ||
>              (chunk_type == SND2_TAG) || (chunk_type == VQFR_TAG)) {
>  
>              ret= av_get_packet(pb, pkt, chunk_size);
> @@ -235,6 +256,32 @@ static int wsvqa_read_packet(AVFormatContext *s,
>                  }
>                  break;
>              case VQFR_TAG:
> +                /* if a new codebook is available inside an earlier a VQFL chunk then
> +                 * prepend it to 'pkt' */
> +                if (wsvqa->vqfl_chunk_size > 0) {
> +                    int64_t current_pos = pkt->pos;
> +                    int current_size = pkt->size;
> +
> +                    if (avio_seek(pb, wsvqa->vqfl_chunk_pos, SEEK_SET) < 0)
> +                        return AVERROR(EIO);
> +
> +                    /* the decoder expects chunks to be 16-bit aligned */
> +                    if (wsvqa->vqfl_chunk_size % 2 == 1)
> +                        wsvqa->vqfl_chunk_size++;
> +
> +                    av_packet_unref(pkt);

This fixes the memleak, yet the main data is still read twice.

> +                    if (av_get_packet(pb, pkt, wsvqa->vqfl_chunk_size) < 0)
> +                        return AVERROR(EIO);
> +
> +                    if (avio_seek(pb, current_pos, SEEK_SET) < 0)
> +                        return AVERROR(EIO);
> +
> +                    ret = av_append_packet(pb, pkt, current_size);
> +
> +                    wsvqa->vqfl_chunk_pos = 0;
> +                    wsvqa->vqfl_chunk_size = 0;
> +                }
> +
>                  pkt->stream_index = wsvqa->video_stream_index;
>                  pkt->duration = 1;
>                  break;
>
Pekka Väänänen Oct. 4, 2021, 3:48 p.m. UTC | #2
Hello,

I've fixed the issues and posted the patch in this same thread:

https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-September/285840.html
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-September/285841.html

Please let me know if there are additional problems with the patch that 
block its merge.
diff mbox series

Patch

diff --git a/libavformat/westwood_vqa.c b/libavformat/westwood_vqa.c
index 77df007c11..cc859c58ca 100644
--- a/libavformat/westwood_vqa.c
+++ b/libavformat/westwood_vqa.c
@@ -1,6 +1,7 @@ 
 /*
  * Westwood Studios VQA Format Demuxer
- * Copyright (c) 2003 The FFmpeg project
+ * Copyright (c) 2003 Mike Melanson <melanson@pcisys.net>
+ * Copyright (c) 2021 Pekka Väänänen <pekka.vaananen@iki.fi>
  *
  * This file is part of FFmpeg.
  *
@@ -30,6 +31,7 @@ 
 
 #include "libavutil/intreadwrite.h"
 #include "avformat.h"
+#include "avio_internal.h"
 #include "internal.h"
 
 #define FORM_TAG MKBETAG('F', 'O', 'R', 'M')
@@ -40,6 +42,7 @@ 
 #define SND1_TAG MKBETAG('S', 'N', 'D', '1')
 #define SND2_TAG MKBETAG('S', 'N', 'D', '2')
 #define VQFR_TAG MKBETAG('V', 'Q', 'F', 'R')
+#define VQFL_TAG MKBETAG('V', 'Q', 'F', 'L')
 
 /* don't know what these tags are for, but acknowledge their existence */
 #define CINF_TAG MKBETAG('C', 'I', 'N', 'F')
@@ -60,6 +63,8 @@  typedef struct WsVqaDemuxContext {
     int sample_rate;
     int audio_stream_index;
     int video_stream_index;
+    int64_t vqfl_chunk_pos;
+    int vqfl_chunk_size;
 } WsVqaDemuxContext;
 
 static int wsvqa_probe(const AVProbeData *p)
@@ -120,6 +125,8 @@  static int wsvqa_read_header(AVFormatContext *s)
     wsvqa->channels     = header[26];
     wsvqa->bps          = header[27];
     wsvqa->audio_stream_index = -1;
+    wsvqa->vqfl_chunk_pos     = 0;
+    wsvqa->vqfl_chunk_size    = 0;
 
     s->ctx_flags |= AVFMTCTX_NOHEADER;
 
@@ -172,7 +179,21 @@  static int wsvqa_read_packet(AVFormatContext *s,
 
         skip_byte = chunk_size & 0x01;
 
-        if ((chunk_type == SND0_TAG) || (chunk_type == SND1_TAG) ||
+        if (chunk_type == VQFL_TAG) {
+            /* Each VQFL chunk carries only a codebook update inside which must be applied
+             * before the next VQFR is rendered. That's why we stash the VQFL offset here
+             * so it can be combined with the next VQFR packet. This way each packet
+             * includes a whole frame as expected. */
+            wsvqa->vqfl_chunk_pos = avio_tell(pb);
+            wsvqa->vqfl_chunk_size = (int)(chunk_size);
+            if (wsvqa->vqfl_chunk_size < 0 || wsvqa->vqfl_chunk_size > 3 * (1 << 20))
+                return AVERROR_INVALIDDATA;
+            /* We need a big seekback buffer because there can be SNxx, VIEW and ZBUF
+             * chunks in the stream before we reach VQFR and seek back here. */
+            ffio_ensure_seekback(pb, wsvqa->vqfl_chunk_size + 512 * 1024);
+            avio_skip(pb, chunk_size + skip_byte);
+            continue;
+        } else if ((chunk_type == SND0_TAG) || (chunk_type == SND1_TAG) ||
             (chunk_type == SND2_TAG) || (chunk_type == VQFR_TAG)) {
 
             ret= av_get_packet(pb, pkt, chunk_size);
@@ -235,6 +256,32 @@  static int wsvqa_read_packet(AVFormatContext *s,
                 }
                 break;
             case VQFR_TAG:
+                /* if a new codebook is available inside an earlier a VQFL chunk then
+                 * prepend it to 'pkt' */
+                if (wsvqa->vqfl_chunk_size > 0) {
+                    int64_t current_pos = pkt->pos;
+                    int current_size = pkt->size;
+
+                    if (avio_seek(pb, wsvqa->vqfl_chunk_pos, SEEK_SET) < 0)
+                        return AVERROR(EIO);
+
+                    /* the decoder expects chunks to be 16-bit aligned */
+                    if (wsvqa->vqfl_chunk_size % 2 == 1)
+                        wsvqa->vqfl_chunk_size++;
+
+                    av_packet_unref(pkt);
+                    if (av_get_packet(pb, pkt, wsvqa->vqfl_chunk_size) < 0)
+                        return AVERROR(EIO);
+
+                    if (avio_seek(pb, current_pos, SEEK_SET) < 0)
+                        return AVERROR(EIO);
+
+                    ret = av_append_packet(pb, pkt, current_size);
+
+                    wsvqa->vqfl_chunk_pos = 0;
+                    wsvqa->vqfl_chunk_size = 0;
+                }
+
                 pkt->stream_index = wsvqa->video_stream_index;
                 pkt->duration = 1;
                 break;