| Message ID | 20210921182502.7807-2-pekka.vaananen@iki.fi |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/westwood_vqa: Store VQFL codebook chunks | expand |
| 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 |
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; >
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 --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;