[PATCH 2/2] avformat/dss: take DSS SP framing from the block headers

Every 512 byte audio block starts with a six byte header, and three of those
bytes describe the framing: how many bytes at the start of the payload still
belong to the frame that began in the previous block, the byte-swap parity of
the first whole frame in this block, and how many frames the block holds. The
demuxer skipped all three and derived the framing instead by running on from
the previous frame, from the first block to the last.

The two agree for as long as nothing disturbs the recording, which is why most
files decode correctly today. But a recording that was paused, resumed or
edited restates the framing at the block where it resumes, and a running walk
does not notice. From that point every frame is read a byte out of phase: the
decoder is handed reflection coefficients and pulse positions that were never
written, and the rest of the file is noise. Ignoring the frame count has a
second effect, milder but always present, of reading on past the last recorded
frame in a block and manufacturing audio that was never in the file.

Start each block where its header says its first frame starts, with the parity
its header gives, and emit exactly as many frames as it declares. Frames still
straddle block boundaries, so the payload read steps over any header it meets.
G.723.1 blocks carry no parity and their frames do not straddle, so that path
is left alone.

Measured against a reference decoder on seven recordings from Olympus and
Philips machines, mean sample correlation goes from 0.5849 to 0.9995. Three
were misframed and went from 0.03, 0.03 and 0.04 to 0.9998, 0.9990 and 0.9997,
with their share of clipped samples falling from around 0.6% to 0.001%.

The other four already decoded well, and the change barely touches them. Three
come out with not one sample different; two of those three also lose a short
tail, 2112 and 1320 samples, of padding the old walk read past the last
declared frame. The fourth turned out to hold a misframed stretch of its own
that the sampled windows had missed: 2% of its samples change, and its clipped
share drops from 0.016% to 0.001%.

On a wider set of 187 recordings, nothing that decoded before stops decoding,
and four files that produced no audio at all now decode. Where honouring the
frame count makes the output shorter than before, the new length agrees to a
tenth of a second with an independent decoder validated against the same
reference, so the samples dropped are ones upstream was inventing.

fate-dss-sp and fate-dss-lp are unchanged and pass: on an undisturbed recording
the block headers say exactly what the running walk assumed, so the output is
bit-identical.
---
 libavformat/dss.c | 122 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 98 insertions(+), 24 deletions(-)

diff --git a/libavformat/dss.c b/libavformat/dss.c
index ea59cb56..2427b34b 100644
--- a/libavformat/dss.c
+++ b/libavformat/dss.c
@@ -52,6 +52,8 @@ typedef struct DSSDemuxContext {
     unsigned int audio_codec;
     int counter;
     int swap;
+    int64_t block_pos;      /* offset of the current audio block header */
+    int frames_left;        /* frames still to come from that block */
     int dss_sp_swap_byte;
 
     int packet_size;
@@ -173,11 +175,16 @@ static int dss_read_header(AVFormatContext *s)
         return (int)ret64;
 
     ctx->counter = 0;
-    ctx->swap    = 0;
+    ctx->swap        = 0;
+    ctx->frames_left = 0;
+    /* dss_sp_next_block steps forward first, so start one block short. */
+    ctx->block_pos   = ctx->dss_header_size - DSS_BLOCK_SIZE;
 
     return 0;
 }
 
+/* G.723.1 blocks carry no byte-swap parity and their frames do not straddle,
+ * so this path keeps the running walk it has always used. */
 static void dss_skip_audio_header(AVFormatContext *s, AVPacket *pkt)
 {
     DSSDemuxContext *ctx = s->priv_data;
@@ -187,6 +194,75 @@ static void dss_skip_audio_header(AVFormatContext *s, AVPacket *pkt)
     ctx->counter += DSS_BLOCK_SIZE - DSS_AUDIO_BLOCK_HEADER_SIZE;
 }
 
+/* Read `size` bytes of frame payload, stepping over any block header met on
+ * the way. Block payloads are contiguous as far as the frames are concerned,
+ * and a frame is short enough to straddle at most one boundary. */
+static int dss_sp_read_payload(AVFormatContext *s, uint8_t *buf, int size)
+{
+    DSSDemuxContext *ctx = s->priv_data;
+    AVIOContext *pb = s->pb;
+    int ret;
+
+    while (size > 0) {
+        int64_t off = (avio_tell(pb) - ctx->dss_header_size) % DSS_BLOCK_SIZE;
+        int chunk;
+
+        if (off < DSS_AUDIO_BLOCK_HEADER_SIZE) {
+            if (avio_skip(pb, DSS_AUDIO_BLOCK_HEADER_SIZE - off) < 0)
+                return AVERROR_EOF;
+            continue;
+        }
+        chunk = FFMIN(size, (int)(DSS_BLOCK_SIZE - off));
+        ret = ffio_read_size(pb, buf, chunk);
+        if (ret < 0)
+            return ret;
+        buf  += chunk;
+        size -= chunk;
+    }
+    return 0;
+}
+
+/* Move to the next audio block and take its framing from its header.
+ *
+ * Each block states the length of the frame fragment carried over from the
+ * previous block, the byte-swap parity of its first whole frame, and how many
+ * frames it holds. Deriving the framing by running on from the previous frame
+ * instead gives the same answer for as long as nothing disturbs the recording,
+ * which is why most files decode; but a paused or edited recording restates
+ * them, and a running walk then reads every remaining frame a byte out of
+ * phase. Believing the block keeps the walk in step, and costs nothing when
+ * the two agree. */
+static int dss_sp_next_block(AVFormatContext *s)
+{
+    DSSDemuxContext *ctx = s->priv_data;
+    AVIOContext *pb = s->pb;
+    uint8_t header[DSS_AUDIO_BLOCK_HEADER_SIZE];
+    int cont, ret;
+
+    for (;;) {
+        ctx->block_pos += DSS_BLOCK_SIZE;
+        if (avio_seek(pb, ctx->block_pos, SEEK_SET) < 0)
+            return AVERROR_EOF;
+        ret = ffio_read_size(pb, header, DSS_AUDIO_BLOCK_HEADER_SIZE);
+        if (ret < 0)
+            return ret;
+
+        ctx->swap = !!(header[0] & 0x80);
+        cont      = 2 * header[1] + 2 * ctx->swap - DSS_AUDIO_BLOCK_HEADER_SIZE;
+        if (cont < 0 || cont > DSS_BLOCK_SIZE - DSS_AUDIO_BLOCK_HEADER_SIZE)
+            cont = 0;
+
+        if (!header[2])         /* an empty block: nothing to emit, move on */
+            continue;
+
+        ctx->frames_left = header[2];
+        if (avio_seek(pb, ctx->block_pos + DSS_AUDIO_BLOCK_HEADER_SIZE + cont,
+                      SEEK_SET) < 0)
+            return AVERROR_EOF;
+        return 0;
+    }
+}
+
 static void dss_sp_byte_swap(DSSDemuxContext *ctx, uint8_t *data)
 {
     int i;
@@ -210,11 +286,15 @@ static void dss_sp_byte_swap(DSSDemuxContext *ctx, uint8_t *data)
 static int dss_sp_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     DSSDemuxContext *ctx = s->priv_data;
-    int read_size, ret, offset = 0, buff_offset = 0;
-    int64_t pos = avio_tell(s->pb);
+    int read_size, ret, buff_offset = 0;
+    int64_t pos;
 
-    if (ctx->counter == 0)
-        dss_skip_audio_header(s, pkt);
+    if (ctx->frames_left <= 0) {
+        ret = dss_sp_next_block(s);
+        if (ret < 0)
+            return ret;
+    }
+    pos = avio_tell(s->pb);
 
     if (ctx->swap) {
         read_size   = DSS_FRAME_SIZE - 2;
@@ -230,22 +310,12 @@ static int dss_sp_read_packet(AVFormatContext *s, AVPacket *pkt)
     pkt->pos = pos;
     pkt->stream_index = 0;
 
-    if (ctx->counter < read_size) {
-        ret = avio_read(s->pb, pkt->data + buff_offset,
-                        ctx->counter);
-        if (ret < ctx->counter)
-            goto error_eof;
-
-        offset = ctx->counter;
-        dss_skip_audio_header(s, pkt);
-    }
-    ctx->counter -= read_size;
-
     /* This will write one byte into pkt's padding if buff_offset == 3 */
-    ret = avio_read(s->pb, pkt->data + offset + buff_offset,
-                    read_size - offset);
-    if (ret < read_size - offset)
-        goto error_eof;
+    ret = dss_sp_read_payload(s, pkt->data + buff_offset, read_size);
+    if (ret < 0)
+        return ret == AVERROR_EOF ? ret : AVERROR_EOF;
+
+    ctx->frames_left--;
 
     dss_sp_byte_swap(ctx, pkt->data);
 
@@ -254,9 +324,6 @@ static int dss_sp_read_packet(AVFormatContext *s, AVPacket *pkt)
     }
 
     return 0;
-
-error_eof:
-    return ret < 0 ? ret : AVERROR_EOF;
 }
 
 static int dss_723_1_read_packet(AVFormatContext *s, AVPacket *pkt)
@@ -346,7 +413,14 @@ static int dss_read_seek(AVFormatContext *s, int stream_index,
     ret = ffio_read_size(s->pb, header, DSS_AUDIO_BLOCK_HEADER_SIZE);
     if (ret < 0)
         return ret;
-    ctx->swap = !!(header[0] & 0x80);
+    ctx->swap        = !!(header[0] & 0x80);
+    ctx->frames_left = 0;
+    ctx->block_pos   = seekto - DSS_BLOCK_SIZE;
+    if (ctx->audio_codec == DSS_ACODEC_DSS_SP) {
+        ctx->dss_sp_swap_byte = -1;
+        return 0;
+    }
+
     offset = 2*header[1] + 2*ctx->swap;
     if (offset < DSS_AUDIO_BLOCK_HEADER_SIZE)
         return AVERROR_INVALIDDATA;
-- 
2.39.5

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]