transcode: refactor to fix decoder/filter/encoder issues after push model and integrate tests
Disclamer: please don't remove the draft and please don't spend too much time reviewing details for now. I feel this is still in an incomplete state.
This is a draft MR because I'm still not 100% confident on the changes. I've kept them for a long time as lack of test and ideas for testing this, and lack of time to apply the ideas when they existed prevented me from ensuring this.
I've spend some time designing a test infrastructure for the transcode, which seems to work well so far, but is still fairly incomplete. In particular, it doesn't really try to check errors, filters and output for now, only decoder, encoder and converter.
I publish this now given that @asenat restarted work from me and mainly @fcartegnie to integrate PCR updates across the stream filter pipeline, and working through such different of the transcode, and without testing, might be a bit demanding on the synchronization cost.
This merge request address three main issues in the current transcode pipeline:
-
1/ The encoder test is not taking video context into account, and doesn't really makes sense once we have all the data for creating the real encoder. It was meant to having the stream
add
operation fails synchronously, but since decoder became asynchronous and is might actually be waiting for data before reporting the new output format, it must also provides asynchronous error reporting (b_error). -
2/ Asynchronous decoders had an issue where their input queue could be filled, blocking the decode() function, while their output queue would wait for a block being release. Output buffers would have been queued in the processing queue of the transcode but would never be reached since the transcode would be stuck in the input
decode
callback from the decoder, which will never be able to return from. -
3/ Lack of testing which leaded to issue hard to track during the push rework. Most points currently tested are related to the push rework though, and it should be improved to include tests against the changes in the current MR.
In addition for 1/, encoder opened is being changed, to make sure the format of the encoder never change and the encoder is not reopened with a potentially different output format and different metadata. Handling this somehow is probably somehow wanted in the future, but this looks more difficult than the general TS case and it looks much less broken to fail rather than create 10 video tracks. This of course still needs testing too.
Note that on master, if you remove the check on encoder open (which obviously fails since there is the encoder test), some scenarios like the sixth one (Scenario 5) is already failing.
{
.source = source_800_600,
.sout = "sout=#transcode:dummy",
.decoder_setup = decoder_i420_800_600_vctx,
.decoder_decode = decoder_decode_vctx,
.encoder_setup = encoder_nv12_800_600,
.encoder_encode = encoder_encode_dummy,
.converter_setup = converter_i420_to_nv12_800_600,
},{
This is a first prerequisite for #25153 and #25152 (closed). What's missing afterwards is a format update callback for encoders and everything would be mergeable. In #25153 and #25152 (closed) implementation, I currently implemented a similar behaviour by adding a packetizer after the encoder, which will be able to parse the stream and extract codec extradata. When the first block is being output by the packetizer, we can check the format and create the downstream ID. This is a workaround too big to merge unfortunately.