More audio

[Asd-g]

No description provided.

[msg7086]

Is this condition correct?

[Asd-g]

With the current decide_audio_seek_method, I think so.

However with the current decide_audio_seek_method if we have packet/frame with position -1 and the seeking mode is POS, the behavior is not reliable. Depending on where this packet is, the result may be:

• seeking reads and discards packets until EOF;
• seeking fails to establish a match;
• seeking falls through with a wrong packet;
• sequential decoding happens to work by packet order;
• or seeking silently misaligns.

[msg7086]

AI flagged this and suggested

if ((pkt->pos == -1 || adhp->frame_list[i].file_offset == -1)
 && pkt->pos != adhp->frame_list[i].file_offset)

[Asd-g]

This can help in case like this:

frame 1: file_offset = -1, keyframe = 0
frame 2: file_offset = -1, keyframe = 0
frame 3: file_offset = 1000, keyframe = 1

But it could cause false mismatch in cases like this:

frame 1: file_offset = -1
frame 2: file_offset = -1
frame 3: file_offset = -1
frame 4: file_offset = 1000

The current check is more conservative and I think it's better because it doesn't assume unknown == unknown => match.

A better solution would be something like:

// lwindex.c

if (adhp->lw_seek_flags & SEEK_POS_BASED) {
    if (lwhp->format_flags & AVFMT_NO_BYTE_SEEK) {
        adhp->lw_seek_flags &= ~SEEK_POS_BASED;
    } else {
        uint32_t real_frame_count = 0;
        uint32_t unknown_pos_count = 0;

        for (uint32_t i = 1; i <= sample_count; i++) {
            if (lw_audio_is_gap_offset(info[i].file_offset))
                continue;

            ++real_frame_count;

            if (!lw_audio_has_valid_file_offset(info[i].file_offset))
                ++unknown_pos_count;
        }

        if (real_frame_count == 0 || unknown_pos_count == real_frame_count) {
            adhp->lw_seek_flags &= ~SEEK_POS_BASED;
        } else if (unknown_pos_count > 0
                   && (adhp->lw_seek_flags & (SEEK_PTS_BASED | SEEK_DTS_BASED))) {
            adhp->lw_seek_flags &= ~SEEK_POS_BASED;
        }
    }
}

This way we change the global seeking mode if there are some packets with unknown position from position seeking to PTS / DTS if these are available. It could be a bit aggressive as it written now. It can be added some threshold - if only one packet has unknown position, then still use position seeking.

[msg7086]

After tracing the full seek path and testing the disputed condition, I agree with the concern here and retract the suggested change.

AVPacket.pos == -1 means that the byte position is unknown; it is not a real position value. A real MPEG-PS sample produced consecutive audio packets like this:

pos=2048
pos=N/A
pos=N/A
pos=N/A
pos=120832

Therefore, -1 == -1 does not establish packet identity.

A minimal counterexample is:

index frame 1: pos=-1
index frame 2: pos=-1
index frame 3: pos=-1
index frame 4: pos=1000

requested frame: 3
actual packet returned after the byte seek: physical frame 2

With the suggested condition, the first runtime packet with pos=-1 is immediately accepted as index frame 1. The observed mapping then becomes:

logical 1 <- physical 2
logical 2 <- physical 3
logical 3 <- physical 4

The call returns physical frame 4 when frame 3 was requested. This is silent audio misalignment. The existing condition instead refuses to claim synchronization from unknown positions; it may fail to produce output in this bad-seek case, but it does not silently identify the wrong packet as the requested frame.

The earlier red/green test was overfitted because its av_seek_frame mock always reset the packet cursor to the first packet. That encoded the unproven assumption that the first pos=-1 packet after seeking must be index frame 1.

There is also a real path to an inaccurate seek. For ordinary seekable files, FFmpeg currently clamps byte offset -1 to data_offset, which often makes this appear to work. However, with non-seekable/custom IO, I reproduced av_seek_frame(..., -1, AVSEEK_FLAG_BYTE) returning success while the underlying read position did not move. FFmpeg's seek_frame_byte() currently does not propagate the return value from avio_seek(). A later pos=-1 packet can therefore satisfy the proposed equality even though it is not the indexed frame.

One important control-flow detail is that this check runs only while match == 0. After synchronization has been established using a valid position, subsequent real packets with pos=-1 are decoded sequentially; the current check does not unconditionally discard all such audio packets.

I therefore think the conservative check should remain. A safer direction is to avoid choosing an unaddressable pos=-1 pre-roll frame unless there is a reliable earlier anchor, and to prefer PTS/DTS seeking when unknown positions make position-based synchronization ambiguous. Unknown-position counts or an unknown == unknown comparison cannot by themselves prove alignment.

Analysis performed in Codex with the user-selected GPT-5.6 Sol model at max thinking level.

[Asd-g]

You're proposing dynamic seeking mode change? If so, I think it's more fragile than changing the global seeking mode. shift_current_frame_number_* functions assume consistent seek method. Dynamic seeking mode will introduce non-determinism.

[msg7086]

Saying I think we can keep existing behavior for now.

[Asd-g]

Some notes about inherited behavior that is not touched:

• when resampling random seeking is not guaranteed to be bit exact to linear decoding;
• lossy codec seeking may produce slightly different PCM compared to linear decoding if sufficient pre-roll / delay metadata is not present in the container;
• the whole audio processing of the plugin is designed for constant rate. Variable rate = unexpected behavior.