PR #24458 opened by superuser404
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24458
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24458.patch

`ff_read_frame_flush()` closes and reopens the parser on every reposition, so 
each
seek hands `vc1_parse()` a zeroed `VC1Context`. `vc1_parser.c` never seeds it 
from
`avctx->extradata`; only `vc1_decode_init()` does that. So after a seek, 
`profile`
reads as simple and `max_coded_width`/`max_coded_height` are zero until an 
in-stream
sequence header happens to pass.

An entry point BDU reaching the parser in that state is read at the wrong bit 
offset:
`hrd_full[]` precedes `coded_size_flag` only when the sequence header set
`hrd_param_flag`, and a zeroed context cannot know that. The bit taken for
`coded_size_flag` is then the first `hrd_full[]` bit, so it comes down to the 
leaky
bucket fullness at that entry point:

* below half full: `coded_size_flag` reads 0, `ff_vc1_decode_entry_point()` 
falls back
  to the zero pair, and `ff_set_dimensions()` fails with

  ```
  [vc1] [IMGUTILS] Picture size 0x0 is invalid
  [vc1] Failed to set dimensions 0 0
  ```

* above half full: a coded size is taken out of the following payload instead, 
silently.

The same zeroed context also sends an advanced profile frame header through the
simple/main reader, so `pict_type`, `repeat_pict` and `field_order` are 
whatever falls
out; `libavformat/demux.c` writes the first two into the packet key flag and 
the packet
duration.

The patch parses extradata once per parser instance, mirroring 
`vc1_decode_init()`.

### Reproduction

>From a public sample, no private media:

```sh
curl -O 
https://samples.ffmpeg.org/V-codecs/WVC1/VC1_interlaced_1080i60_with_artifacts_crashes.mkv
ffmpeg -i VC1_interlaced_1080i60_with_artifacts_crashes.mkv -map 0:v:0 -c copy 
-f data traviata.vc1
python3 gen_vc1_seek_repro.py traviata.vc1 repro.vc1
ffmpeg -r 30000/1001 -f vc1 -i repro.vc1 -c copy repro.mkv
ffprobe -read_intervals "2.1%+#4" repro.mkv
```

The sample as published repeats the sequence header at every entry point, which 
keeps
the parser seeded, and its `hrd_full[0]` happens to sit above half. The 
generator makes
the two edits that occur in the wild but not in this particular file: it drops 
the
repeated sequence headers (encoders that rely on the container's extradata emit 
exactly
that) and clears the top bit of `hrd_full[0]`.

```python
#!/usr/bin/env python3
"""Build a VC-1 seek reproducer from a public sample.

Input:  the video elementary stream of
        
samples.ffmpeg.org/V-codecs/WVC1/VC1_interlaced_1080i60_with_artifacts_crashes.mkv
Output: the same stream with two edits, both of which occur in the wild:
        1. the sequence header repeated at each entry point is dropped, so a 
seek
           landing there reaches the parser with nothing to seed it (encoders 
that
           rely on the container's extradata emit exactly this);
        2. the top bit of hrd_full[0] in those entry points is cleared, i.e. the
           leaky bucket is below half full, which is what a zeroed parse context
           mistakes for coded_size_flag == 0.
"""
import sys

src, dst = sys.argv[1], sys.argv[2]
d = bytearray(open(src, 'rb').read())

def bdus(buf):
    out, i = [], 0
    while True:
        j = buf.find(b'\x00\x00\x01', i)
        if j < 0:
            return out
        out.append((j, buf[j + 3]))
        i = j + 3

# 1. drop every sequence header BDU except the one at the head
drop = []
for k, (off, typ) in enumerate(bdus(d)):
    if typ == 0x0F and off > 200:
        drop.append((off, bdus(d)[k + 1][0]))
out, prev = bytearray(), 0
for a, b in drop:
    out += d[prev:a]
    prev = b
out += d[prev:]
d = out

# 2. clear hrd_full[0] MSB in the entry points past the head
for off, typ in bdus(d):
    if typ == 0x0E and off > 200:
        d[off + 5] &= ~0x04

open(dst, 'wb').write(bytes(d))
print(f"{src} -> {dst}: dropped {len(drop)} sequence headers")
```

### Measurements

Six seek points (1.1, 1.4, 1.7, 2.1, 2.4, 2.7 s) on `repro.mkv`:

| | `Picture size 0x0 is invalid` |
|---|---|
| master | 6 of 6 |
| with this patch | 0 of 6 |

A linear read is byte identical either way: `ffprobe -show_entries 
packet=flags,pts,dts,duration`
matches on `repro.mkv`, on the unmodified sample, on the raw elementary stream, 
and on
`vc1/SA10143.vc1` and `vc1/ilaced_twomv.vc1` from the FATE suite.

All nine vc1 FATE tests pass (`fate-vc1_sa00040`, `_sa00050`, `_sa10091`, 
`_sa10143`,
`_sa20021`, `_ilaced_twomv`, `fate-vc1test_smm0005`, `_smm0015`, 
`fate-vc1-ism`).

A track whose frames carry no start codes is unaffected, since the parser reads 
no
header there in the first place.



>From 7f5464dc4375d414fae3a8d5a22fd389f59f7d76 Mon Sep 17 00:00:00 2001
From: Vincent Herbst <[email protected]>
Date: Fri, 11 Sep 2026 12:40:23 +0200
Subject: [PATCH] avcodec/vc1_parser: seed the parse context from extradata

libavformat closes and reopens the parser on every reposition
(ff_read_frame_flush()), so each seek hands vc1_parse() a zeroed
VC1Context: profile reads as simple, and max_coded_width and
max_coded_height as zero, until an in-stream sequence header happens to
pass.

An entry point BDU reaching the parser in that state is read at the wrong
bit offset, because hrd_full[] precedes coded_size_flag only when the
sequence header set hrd_param_flag, which a zeroed context cannot know.
The bit taken for coded_size_flag is then the first hrd_full[] bit, so an
entry point whose leaky bucket is below half full falls back to the zero
pair and ff_set_dimensions() fails:

    [vc1] [IMGUTILS] Picture size 0x0 is invalid
    [vc1] Failed to set dimensions 0 0

while one above half full takes a coded size out of the following
payload. The same zeroed context also sends an advanced profile frame
header through the simple/main reader, so pict_type, repeat_pict and
field_order are whatever falls out of it; libavformat writes the first
two into the packet key flag and the packet duration.

Parse extradata once per parser instance, the way vc1_decode_init() does.
A track whose frames carry no start codes is unaffected, and so is one
whose landing packets repeat the sequence header; linear reads are byte
identical either way.
---
 libavcodec/vc1_parser.c | 62 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/libavcodec/vc1_parser.c b/libavcodec/vc1_parser.c
index 595d066dea..119fd6ab6b 100644
--- a/libavcodec/vc1_parser.c
+++ b/libavcodec/vc1_parser.c
@@ -27,6 +27,7 @@
 
 #include "libavutil/attributes.h"
 #include "libavutil/avassert.h"
+#include "libavutil/mem.h"
 #include "parser.h"
 #include "parser_internal.h"
 #include "vc1.h"
@@ -54,6 +55,7 @@ typedef struct VC1ParseContext {
     ParseContext pc;
     VC1Context v;
     uint8_t prev_start_code;
+    uint8_t extradata_parsed;
     size_t bytes_to_skip;
     uint8_t unesc_buffer[UNESCAPED_LIMIT];
     size_t unesc_index;
@@ -125,6 +127,60 @@ static void vc1_extract_header(AVCodecParserContext *s, 
AVCodecContext *avctx,
     }
 }
 
+/**
+ * Seed the parse context from extradata, the way the decoder does at init.
+ *
+ * libavformat closes and reopens the parser on every reposition
+ * (ff_read_frame_flush()), so each seek starts from a zeroed VC1Context: 
profile
+ * reads as simple, and max_coded_width/max_coded_height as zero, until an
+ * in-stream sequence header happens to pass. An entry point reaching a 
context in
+ * that state is read at the wrong bit offset, because whether hrd_full[] 
precedes
+ * coded_size_flag is a property of the sequence header, and the size it then
+ * falls back to is the zero pair.
+ */
+static void vc1_parse_extradata(AVCodecParserContext *s, AVCodecContext *avctx)
+{
+    VC1ParseContext *vpc = s->priv_data;
+    const uint8_t *start, *end, *next;
+    uint8_t *buf2;
+    GetBitContext gb;
+
+    if (!avctx->extradata || avctx->extradata_size < 16)
+        return;
+
+    buf2 = av_mallocz(avctx->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
+    if (!buf2)
+        return;
+
+    vpc->v.s.avctx = avctx;
+    end   = avctx->extradata + avctx->extradata_size;
+    start = find_next_marker(avctx->extradata, end);
+    for (next = start; next < end; start = next) {
+        int size, buf2_size;
+
+        next = find_next_marker(start + 4, end);
+        size = next - start - 4;
+        if (size <= 0)
+            continue;
+        buf2_size = vpc->v.vc1dsp.vc1_unescape_buffer(start + 4, size, buf2);
+        if (init_get_bits8(&gb, buf2, buf2_size) < 0)
+            break;
+        switch (AV_RB32(start)) {
+        case VC1_CODE_SEQHDR:
+            if (ff_vc1_decode_sequence_header(avctx, &vpc->v, &gb) < 0)
+                goto done;
+            break;
+        case VC1_CODE_ENTRYPOINT:
+            if (ff_vc1_decode_entry_point(avctx, &vpc->v, &gb) < 0)
+                goto done;
+            break;
+        }
+    }
+
+done:
+    av_free(buf2);
+}
+
 static int vc1_parse(AVCodecParserContext *s,
                            AVCodecContext *avctx,
                            const uint8_t **poutbuf, int *poutbuf_size,
@@ -142,6 +198,11 @@ static int vc1_parse(AVCodecParserContext *s,
     int next = END_NOT_FOUND;
     int i = vpc->bytes_to_skip;
 
+    if (!vpc->extradata_parsed) {
+        vpc->extradata_parsed = 1;
+        vc1_parse_extradata(s, avctx);
+    }
+
     if (pic_found && buf_size == 0) {
         /* EOF considered as end of frame */
         memset(unesc_buffer + unesc_index, 0, UNESCAPED_THRESHOLD - 
unesc_index);
@@ -265,6 +326,7 @@ static av_cold int vc1_parse_init(AVCodecParserContext *s)
     vpc->v.first_pic_header_flag = 1;
     vpc->v.parse_only = 1;
     vpc->prev_start_code = 0;
+    vpc->extradata_parsed = 0;
     vpc->bytes_to_skip = 0;
     vpc->unesc_index = 0;
     vpc->search_state = NO_MATCH;
-- 
2.52.0

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

Reply via email to