Skip to content

Rework packet finding logic #468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arch1t3cht wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Rework packet finding logic #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d349b6
Use RAII wrapper for AVPackets
arch1t3cht Nov 4, 2025
8c8274e
Return a full AVPacket in DecodeNextFrame
arch1t3cht Nov 4, 2025
9c943eb
Rework packet finding functions
arch1t3cht Dec 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 14 additions & 20 deletions src/core/audiosource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,9 @@ FFMS_AudioSource::AudioBlock *FFMS_AudioSource::CacheBlock(CacheIterator &pos) {
}

int FFMS_AudioSource::DecodeNextBlock(CacheIterator *pos) {
AVPacket *Packet = av_packet_alloc();
if (!Packet)
throw FFMS_Exception(FFMS_ERROR_PARSER, FFMS_ERROR_ALLOCATION_FAILED,
"Could not allocate packet.");
if (!ReadPacket(Packet)) {
av_packet_free(&Packet);
SmartAVPacket Packet;

if (!ReadPacket(*Packet)) {
throw FFMS_Exception(FFMS_ERROR_PARSER, FFMS_ERROR_UNKNOWN,
"ReadPacket unexpectedly failed to read a packet");
}
Expand All @@ -315,7 +312,7 @@ int FFMS_AudioSource::DecodeNextBlock(CacheIterator *pos) {
CurrentSample = CurrentFrame->SampleStart;

// Value code intentionally ignored, combined with the checks when indexing this mostly gives the expected behavior
avcodec_send_packet(CodecContext, Packet);
avcodec_send_packet(CodecContext, Packet.get());

int NumberOfSamples = 0;
AudioBlock *CachedBlock = nullptr;
Expand All @@ -331,21 +328,18 @@ int FFMS_AudioSource::DecodeNextBlock(CacheIterator *pos) {
}
break;
} else if (Ret == AVERROR(EAGAIN)) {
if (!ReadPacket(Packet)) {
av_packet_free(&Packet);
if (!ReadPacket(*Packet)) {
throw FFMS_Exception(FFMS_ERROR_PARSER, FFMS_ERROR_UNKNOWN,
"ReadPacket unexpectedly failed to read a packet");
}
avcodec_send_packet(CodecContext, Packet);
avcodec_send_packet(CodecContext, Packet.get());
} else if (Ret == AVERROR_EOF) {
break;
} else {
throw FFMS_Exception(FFMS_ERROR_CODEC, FFMS_ERROR_DECODING, "Audio decoding error");
}
}

av_packet_free(&Packet);

// Zero sample packets aren't included in the index
if (!NumberOfSamples)
return NumberOfSamples;
Expand Down Expand Up @@ -553,24 +547,24 @@ void FFMS_AudioSource::Seek() {
}
}

bool FFMS_AudioSource::ReadPacket(AVPacket *Packet) {
while (av_read_frame(FormatContext, Packet) >= 0) {
if (Packet->stream_index == TrackNumber) {
bool FFMS_AudioSource::ReadPacket(AVPacket &Packet) {
while (av_read_frame(FormatContext, &Packet) >= 0) {
if (Packet.stream_index == TrackNumber) {
// Required because not all audio packets, especially in ogg, have a pts. Use the previous valid packet's pts instead.
if (Packet->pts == AV_NOPTS_VALUE)
Packet->pts = LastValidTS;
if (Packet.pts == AV_NOPTS_VALUE)
Packet.pts = LastValidTS;
else
LastValidTS = Packet->pts;
LastValidTS = Packet.pts;

// This only happens if a really shitty demuxer seeks to a packet without pts *hrm* ogg *hrm* so read until a valid pts is reached
int64_t PacketTS = Frames.HasTS ? Packet->pts : Packet->pos;
int64_t PacketTS = Frames.HasTS ? Packet.pts : Packet.pos;
if (PacketTS != AV_NOPTS_VALUE) {
while (PacketNumber > 0 && FrameTS(PacketNumber) > PacketTS) --PacketNumber;
while (FrameTS(PacketNumber) < PacketTS) ++PacketNumber;
return true;
}
}
av_packet_unref(Packet);
av_packet_unref(&Packet);
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/audiosource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ struct FFMS_AudioSource {
// Called after seeking
void Seek();
// Read the next packet from the file
bool ReadPacket(AVPacket *);
bool ReadPacket(AVPacket &);

// Close and reopen the source file to seek back to the beginning. Only
// needs to do anything for formats that can't seek to the beginning otherwise.
Expand Down
48 changes: 21 additions & 27 deletions src/core/indexing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,10 @@ FFMS_Indexer::FFMS_Indexer(const char *Filename, const FFMS_KeyValuePair *Demuxe
}
}

uint32_t FFMS_Indexer::IndexAudioPacket(int Track, AVPacket *Packet, SharedAVContext &Context, FFMS_Index &TrackIndices) {
uint32_t FFMS_Indexer::IndexAudioPacket(int Track, const AVPacket &Packet, SharedAVContext &Context, FFMS_Index &TrackIndices) {
AVCodecContext *CodecContext = Context.CodecContext;
int64_t StartSample = Context.CurrentSample;
int Ret = avcodec_send_packet(CodecContext, Packet);
int Ret = avcodec_send_packet(CodecContext, &Packet);
if (Ret != 0) {
if (ErrorHandling == FFMS_IEH_ABORT) {
throw FFMS_Exception(FFMS_ERROR_CODEC, FFMS_ERROR_DECODING, "Audio decoding error");
Expand Down Expand Up @@ -364,7 +364,7 @@ void FFMS_Indexer::CheckAudioProperties(int Track, AVCodecContext *Context) {
}
}

void FFMS_Indexer::ParseVideoPacket(SharedAVContext &VideoContext, AVPacket *pkt, int *RepeatPict,
void FFMS_Indexer::ParseVideoPacket(SharedAVContext &VideoContext, const AVPacket &pkt, int *RepeatPict,
int *FrameType, bool *Invisible, bool *SecondField, enum AVPictureStructure *LastPicStruct) {
if (VideoContext.Parser) {
uint8_t *OB;
Expand All @@ -373,8 +373,8 @@ void FFMS_Indexer::ParseVideoPacket(SharedAVContext &VideoContext, AVPacket *pkt
av_parser_parse2(VideoContext.Parser,
VideoContext.CodecContext,
&OB, &OBSize,
pkt->data, pkt->size,
pkt->pts, pkt->dts, pkt->pos);
pkt.data, pkt.size,
pkt.pts, pkt.dts, pkt.pos);

// H.264 (PAFF) and HEVC may have one field per packet, so we need to track
// when we have a full or half frame available, and mark one of them as
Expand All @@ -394,15 +394,15 @@ void FFMS_Indexer::ParseVideoPacket(SharedAVContext &VideoContext, AVPacket *pkt

*RepeatPict = VideoContext.Parser->repeat_pict;
*FrameType = VideoContext.Parser->pict_type;
*Invisible = (VideoContext.Parser->repeat_pict < 0 || (pkt->flags & AV_PKT_FLAG_DISCARD));
*Invisible = (VideoContext.Parser->repeat_pict < 0 || (pkt.flags & AV_PKT_FLAG_DISCARD));
} else {
*Invisible = !!(pkt->flags & AV_PKT_FLAG_DISCARD);
*Invisible = !!(pkt.flags & AV_PKT_FLAG_DISCARD);
}

if (VideoContext.CodecContext->codec_id == AV_CODEC_ID_VP8)
ParseVP8(pkt->data[0], Invisible, FrameType);
ParseVP8(pkt.data[0], Invisible, FrameType);
else if (VideoContext.CodecContext->codec_id == AV_CODEC_ID_VP9)
ParseVP9(pkt->data[0], FrameType);
ParseVP9(pkt.data[0], FrameType);
}

void FFMS_Indexer::Free() {
Expand Down Expand Up @@ -502,34 +502,30 @@ FFMS_Index *FFMS_Indexer::DoIndexing() {
}
}

AVPacket *Packet = av_packet_alloc();
if (!Packet)
throw FFMS_Exception(FFMS_ERROR_CODEC, FFMS_ERROR_ALLOCATION_FAILED,
"Could not allocate packet.");
SmartAVPacket Packet;
std::vector<int64_t> LastValidTS(FormatContext->nb_streams, AV_NOPTS_VALUE);

int64_t filesize = avio_size(FormatContext->pb);
enum AVPictureStructure LastPicStruct = AV_PICTURE_STRUCTURE_UNKNOWN;
int ret;
while ((ret = av_read_frame(FormatContext, Packet)) >= 0) {
while ((ret = av_read_frame(FormatContext, Packet.get())) >= 0) {
// Update progress
// FormatContext->pb can apparently be NULL when opening images.
if (IC && FormatContext->pb) {
if ((*IC)(FormatContext->pb->pos, filesize, ICPrivate)) {
av_packet_free(&Packet);
throw FFMS_Exception(FFMS_ERROR_CANCELLED, FFMS_ERROR_USER,
"Cancelled by user");
}
}
if (!IndexMask.count(Packet->stream_index)) {
av_packet_unref(Packet);
av_packet_unref(Packet.get());
continue;
}

int Track = Packet->stream_index;
FFMS_Track &TrackInfo = (*TrackIndices)[Track];
bool KeyFrame = !!(Packet->flags & AV_PKT_FLAG_KEY);
ReadTS(Packet, LastValidTS[Track], (*TrackIndices)[Track].UseDTS);
ReadTS(*Packet, LastValidTS[Track], (*TrackIndices)[Track].UseDTS);

if (FormatContext->streams[Track]->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
int64_t PTS = TrackInfo.UseDTS ? Packet->dts : Packet->pts;
Expand All @@ -545,7 +541,6 @@ FFMS_Index *FFMS_Indexer::DoIndexing() {
bool HasAltRefs = (FormatContext->streams[Track]->codecpar->codec_id == AV_CODEC_ID_VP8 ||
FormatContext->streams[Track]->codecpar->codec_id == AV_CODEC_ID_VP9);
if (Packet->duration == 0 && !HasAltRefs) {
av_packet_free(&Packet);
throw FFMS_Exception(FFMS_ERROR_INDEXING, FFMS_ERROR_PARSER,
"Invalid packet pts, dts, and duration");
}
Expand All @@ -562,7 +557,7 @@ FFMS_Index *FFMS_Indexer::DoIndexing() {
int FrameType = 0;
bool Invisible = false;
bool SecondField = false;
ParseVideoPacket(AVContexts[Track], Packet, &RepeatPict, &FrameType, &Invisible, &SecondField, &LastPicStruct);
ParseVideoPacket(AVContexts[Track], *Packet, &RepeatPict, &FrameType, &Invisible, &SecondField, &LastPicStruct);

TrackInfo.AddVideoFrame(PTS, Packet->dts, RepeatPict, KeyFrame,
FrameType, Packet->pos, Invisible, SecondField);
Expand All @@ -574,7 +569,7 @@ FFMS_Index *FFMS_Indexer::DoIndexing() {
TrackInfo.HasTS = true;

int64_t StartSample = AVContexts[Track].CurrentSample;
uint32_t SampleCount = IndexAudioPacket(Track, Packet, AVContexts[Track], *TrackIndices);
uint32_t SampleCount = IndexAudioPacket(Track, *Packet, AVContexts[Track], *TrackIndices);
TrackInfo.SampleRate = AVContexts[Track].CodecContext->sample_rate;

TrackInfo.AddAudioFrame(LastValidTS[Track], Packet->dts,
Expand All @@ -584,9 +579,8 @@ FFMS_Index *FFMS_Indexer::DoIndexing() {
if (!(Packet->flags & AV_PKT_FLAG_DISCARD))
TrackInfo.LastDuration = Packet->duration;

av_packet_unref(Packet);
av_packet_unref(Packet.get());
}
av_packet_free(&Packet);
if (IsIOError(ret))
throw FFMS_Exception(FFMS_ERROR_INDEXING, FFMS_ERROR_FILE_READ,
"Indexing failed: " + AVErrorToString(ret));
Expand All @@ -595,11 +589,11 @@ FFMS_Index *FFMS_Indexer::DoIndexing() {
return TrackIndices.release();
}

void FFMS_Indexer::ReadTS(const AVPacket *Packet, int64_t &TS, bool &UseDTS) {
if (!UseDTS && Packet->pts != AV_NOPTS_VALUE)
TS = Packet->pts;
void FFMS_Indexer::ReadTS(const AVPacket &Packet, int64_t &TS, bool &UseDTS) {
if (!UseDTS && Packet.pts != AV_NOPTS_VALUE)
TS = Packet.pts;
if (TS == AV_NOPTS_VALUE)
UseDTS = true;
if (UseDTS && Packet->dts != AV_NOPTS_VALUE)
TS = Packet->dts;
if (UseDTS && Packet.dts != AV_NOPTS_VALUE)
TS = Packet.dts;
}
6 changes: 3 additions & 3 deletions src/core/indexing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ struct FFMS_Indexer {
int64_t Filesize;
uint8_t Digest[20];

void ReadTS(const AVPacket *Packet, int64_t &TS, bool &UseDTS);
void ReadTS(const AVPacket &Packet, int64_t &TS, bool &UseDTS);
void CheckAudioProperties(int Track, AVCodecContext *Context);
uint32_t IndexAudioPacket(int Track, AVPacket *Packet, SharedAVContext &Context, FFMS_Index &TrackIndices);
void ParseVideoPacket(SharedAVContext &VideoContext, AVPacket *pkt, int *RepeatPict, int *FrameType, bool *Invisible, bool *SecondField, enum AVPictureStructure *LastPicStruct);
uint32_t IndexAudioPacket(int Track, const AVPacket &Packet, SharedAVContext &Context, FFMS_Index &TrackIndices);
void ParseVideoPacket(SharedAVContext &VideoContext, const AVPacket &pkt, int *RepeatPict, int *FrameType, bool *Invisible, bool *SecondField, enum AVPictureStructure *LastPicStruct);
void Free();
public:
FFMS_Indexer(const char *Filename, const FFMS_KeyValuePair *DemuxerOptions, int NumOptions);
Expand Down
86 changes: 73 additions & 13 deletions src/core/track.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "indexing.h"

#include <algorithm>
#include <array>
#include <cmath>
#include <cstdlib>

Expand Down Expand Up @@ -157,23 +158,82 @@ static bool PTSComparison(FrameInfo FI1, FrameInfo FI2) {
return FI1.PTS < FI2.PTS;
}

int FFMS_Track::FrameFromPTS(int64_t PTS, bool AllowHidden) const {
enum class AVPacketProp {
TS, // can be PTS or DTS depending on UseDTS
Pos,
Hidden,
Key,
};

const std::array<std::vector<AVPacketProp>, 5> FindPacketCheckSequence = {{
{AVPacketProp::TS, AVPacketProp::Pos, AVPacketProp::Hidden, AVPacketProp::Key},
{AVPacketProp::TS, AVPacketProp::Pos, AVPacketProp::Hidden},
{AVPacketProp::TS, AVPacketProp::Pos},
{AVPacketProp::TS},
{AVPacketProp::Pos},
}};

// Attempts to find the given AVPacket in the track's list of FrameInfos,
// returning the FrameInfo's index if one was found and -1 if not.
//
// The finding logic begins by trying to find a *unique* FrameInfo whose
// PTS, Pos, and flags match the given packet.
// If no such packet exists, it tries a fallback sequence of comparing fewer fields.
//
// The current fallback sequence is mostly chosen based on intuition - for
// well-behaved files simply checking all fields should work fine.
// If examples come up where this fallback sequence becomes relevant, it can
// be adjusted.
int FFMS_Track::FindPacket(const AVPacket &packet) const {
FrameInfo F;
F.PTS = PTS;
F.PTS = UseDTS ? packet.dts : packet.pts;

auto SameTSBegin = std::lower_bound(begin(), end(), F, PTSComparison);;
auto SameTSEnd = SameTSBegin;
while (SameTSEnd != end() && SameTSEnd->PTS == F.PTS)
SameTSEnd++;

for (auto const& checks : FindPacketCheckSequence) {
// If the check sequence includes the timestamp, we can begin with a binary search
// since the FrameInfos are sorted by their timestamp, and then do a linear search in the found interval.
// If the check sequence does not include the timestamp, we have to do a full linear search.
auto Begin = begin();
auto End = end();

if (checks[0] == AVPacketProp::TS) {
Begin = SameTSBegin;
End = SameTSEnd;
}

auto Pos = std::lower_bound(begin(), end(), F, PTSComparison);
while (Pos != end() && (!AllowHidden && Pos->Skipped()) && Pos->PTS == PTS)
Pos++;
int found = 0;
int result = -1;

for (auto it = Begin; it < End; it++) {
bool match = std::all_of(checks.cbegin(), checks.cend(), [&](AVPacketProp check) {
switch (check) {
case AVPacketProp::TS:
return it->PTS == F.PTS;
case AVPacketProp::Pos:
return it->FilePos == packet.pos;
case AVPacketProp::Hidden:
return it->MarkedHidden == (packet.flags & AV_PKT_FLAG_DISCARD);
case AVPacketProp::Key:
return it->KeyFrame == (packet.flags & AV_PKT_FLAG_KEY);
}
return false;
});

if (match) {
found++;
result = std::distance(begin(), it);
}
}

if (Pos == end() || Pos->PTS != PTS)
return -1;
return std::distance(begin(), Pos);
}
if (found == 1) {
return result;
}
}

int FFMS_Track::FrameFromPos(int64_t Pos, bool AllowHidden) const {
for (size_t i = 0; i < size(); i++)
if (Data->Frames[i].FilePos == Pos && (AllowHidden || !Data->Frames[i].Skipped()))
return static_cast<int>(i);
return -1;
}

Expand Down
4 changes: 2 additions & 2 deletions src/core/track.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <vector>
#include <memory>

struct AVPacket;
class ZipFile;

struct FrameInfo {
Expand Down Expand Up @@ -82,8 +83,7 @@ struct FFMS_Track {
void FillAudioGaps();

int FindClosestVideoKeyFrame(int Frame) const;
int FrameFromPTS(int64_t PTS, bool AllowHidden = false) const;
int FrameFromPos(int64_t Pos, bool AllowHidden = false) const;
int FindPacket(const AVPacket &packet) const;
int ClosestFrameFromPTS(int64_t PTS) const;
int RealFrameNumber(int Frame) const;
int VisibleFrameCount() const;
Expand Down
12 changes: 12 additions & 0 deletions src/core/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ void FillAP(FFMS_AudioProperties &AP, AVCodecContext *CTX, FFMS_Track &Frames);

void LAVFOpenFile(const char *SourceFile, AVFormatContext *&FormatContext, int Track, const std::map<std::string, std::string> &LAVFOpts);

// RAII wrapper for AVPacket * that handles av_packet_alloc and av_packet_free.
// av_packet_ref and av_packet_unref still need to be handled by user code.
class SmartAVPacket : public std::unique_ptr<AVPacket, void(*)(AVPacket *)> {
Copy link
Member

@dwbuiten dwbuiten Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is everywhere this used actually exception safe? I ask because now there is no longer an explicit alloc failure check, and the unfortunate "exceptions as flow control" history of FFMS (and C++ in general) comes into play.

Copy link
Contributor Author

@arch1t3cht arch1t3cht Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is at the moment because every instance of av_packet_alloc that was replaced had a similar check for the resulting pointer being null and would throw the same FFMS_Exception if it was, so this does not change any exception behavior.

This is why I moved the failure check into the constructor to cut down on some boilerplate, but I see how this is now less explicit about control flow in the case of allocation failure. If you prefer being more explicit about where these exceptions are thrown, I can revert this.

public:
SmartAVPacket() : std::unique_ptr<AVPacket, void(*)(AVPacket *)>(av_packet_alloc(), [](AVPacket *packet) { av_packet_free(&packet); }) {
if (!*this) {
throw FFMS_Exception(FFMS_ERROR_DECODING, FFMS_ERROR_ALLOCATION_FAILED,
"Could not allocate packet.");
}
}
};

namespace optdetail {
template<typename T>
T get_av_opt(void *v, const char *name) {
Expand Down
Loading
Loading