Another idea might be

Extend Piece so that it has

vector<Content>
vector<Decoder>

where there are one or more of each, 1 decoder per 1 content.

Then extend the Piece API so that it does everything currently done by calls into the content from Player.

Content would be:

• all the same type
• only with start trim on 1st content and end trim on last
• some settings the same

Tricky bits with this are:

• _stream_states and audio streams in general
• re-use of old decoders (optimisation)

_stream_states are piece, last_push_end (DCPTime) for every stream in every piece of content Initialised to content->position() in setup_pieces_unlocked

last_push_end updated when audio is pushed into the merger; here the piece and stream are known. Checked when deciding the time before which we have all audio

_stream_states could be a vector in Piece, then it is managed by Piece; call it when audio is pushed, it finds the correct stream from its internal list of content, then updates. Then you could ask it what the earliest stream is.

Going reasonably OK in 1771-resamples-glitches-take3

Probably easiest to manually rebase -take3 onto v2.15.x to remember what is going on.

Made -take4 branch but didn't do anything yet.

Did some work in -take4 but this is starting to feel like a very long way round; even setting Piece up to have multiple content takes a lot, and that is before even setting up the Pieces to take advantage of this.

The AudioDecoder has a Resampler. Say you have two pieces of content that are being resampled but should connect seamlessly:

/-------\/-----\
| 1     | 2    |
\-------/\-----/

When 1's decoder finishes it flushes its resampler, 2 makes a new one. This causes a slight discontinuity.

==> move the Resampler somewhere else (e.g. the player) - it used to be like this until fa4d252

==> keep the same resampler

When 1 finishes maybe it doesn't flush its resampler. Without the flush we actually emit audio up to a bit before the end of 1, then start decoding 2 and pushing its samples into the same resampler. Then stuff comes out of the resampler and this stuff should be at a time just before 2 starts, so the player discards it (since it's outside the bounds of content 2).

==> move this discarding of data back a bit

Player::audio's data comes from AudioDecoder::Data emitting - so can't we stop that emitting out-of-bounds data? But the data would also be out of bounds as far as AudioDecoder is concerned, because it only knows about the one piece of content.

==> somehow coalesce the content or make it look coalesced

take4 does this by changing Piece to allow multiple content with a single "special" decoder which coalesces the output of multiple "real" decoders so that their output ends up in a single AudioDecoder and hence a
single resampler. But this "special" decoder is awkward as it has to present the Decoder interface and proxy things down to sub-decoders.

===> coalesce the content at a different level, e.g. make the existing decoder able to take multiple files
as input

...but you need a decoder for each one to discard the headers and get to the actual samples.
...and Piece is the obvious place to coalesce, since this is a data structure set up just for playing,
invisible to the user.

To maintain the Player::audio semantics of "discard outside Piece::{position,end}" Piece must have multiple bits of content. But Player also decides what to pass() based on Pieces, so that means the piece must manage multiple decoders (or have a proxy decoder which does).

The real question perhaps is, even after this all this preparation, where will the Resampler end up?

===> just have one AudioDecoder shared between multiple (e.g.) FFmpegDecoders?

But Resamplers are managed one-per-stream; when the AD sees data from a stream it hasn't seen before it makes a Resampler.

===> Move Resampler into Piece?

Piece would need _positions. Maybe workable?

===> Just noticed the stuff in FFmpegDecoder::flush to round lengths up to the next video frame and so on. Then the resamplers are flushed... dubious... but maybe OK (the video frame round-up will have to go).

AudioDecoder would not resample; would store stream positions in content frames.
Piece would probably have to have an Audio emission that the player attaches to, and intercept the one from AudioDecoder, resample it with one resampler and re-emit.

Piece soon becomes a sort of decoder; e.g. when the last of its decoders flushes it needs to flush the resamplers; Player must listen to Piece::Audio for audio data rather than decoder->audio->Data;
suggests going back to the grouped-decoder class, but then you need some AudioDecoders that have resamplers and some that do not.

Though in fact it's probably neater if Player listents to Piece::Video, Piece::Text etc., so that might be OK. Piece becomes some content with some decoders merging that content and presenting
data to Player. Maybe Content{Video,Audio,Text} becomes Piece{Video,Audio,Text} and decoders just emit their data without a wrapper.

More and more this seems like a crazy amount of work for a relatively small bug; to trigger it requires not only post-production at the wrong frame rate (as if the content were not intended for DCP) but also in reels (as if it were for DCP).

There's also a high risk of things breaking as a result.

Pros of merging before 2.16.0:

• might get more testing
• it will get done rather than bit-rotting
• there are perhaps some nice fixes / structural improvements as part of the patch
• it will be easier to rebase 2.17.x onto 2.16.x in the early part of development

Cons:

• it might break things
• it's taking ages
• merging it onto 2.17.x will make the branches diverge early

Although if the idea is to release 2.18.0 soon it may not be so bad if there's a divergence.

State as of now is that most tests pass; one notable exception is content_test6.
This is the one which checks how we handle large audio timestamp discontinuities. AudioDecoder resets its position if the timestamp coming from the decoder is more than 1 video frame away from where it "should be". Now, though, that reset is passed onto the Piece but then ignored.

It seems like we should do the stuff in AudioDecoder::emit in Piece instead. And maybe that implies that AudioDecoder should not keep _position at all, and instead of Piece::decoder_before calling out to the decoders to ask what is going on, it should know itself;
for audio it is already counting position in the streams and perhaps it should do the same for video and subs. Or maybe we could special-case audio and do that ourselves.

Either way this definitely feels now like far too much for 2.16.0.