Skip to content
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.

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

StreamBuffer for input #351

Open
wants to merge 2 commits into
base: ma_refactoring_second
Choose a base branch
from

Conversation

MikeIndiaAlpha
Copy link
Contributor

Alternative input system for LPMS. The idea is to allow using of
FFmpeg demultiplexers with any input mechanism, such as network
packet delivery. In order to do that we have to provide custom
AVIOContext with custom read() and seek() operations. This also
means that synchronization is needed because we will have to
perform input and transcoding in different threads (for example
demux - which is a part of transcode - may want more data which
is not available yet, then transcode should block until input
thread delivers more data)

Copy link
Contributor

@AlexKordic AlexKordic left a comment

Choose a reason for hiding this comment

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

Well Done.
Is loadInputBuffer() planned to be removed in future PR?
How do you run tests?

ffmpeg/stream_buffer.c Outdated Show resolved Hide resolved
Comment on lines +61 to +63
void buffer_put_bytes(StreamBuffer *buffer, uint8_t *bytes, int64_t size);
void buffer_end_of_stream(StreamBuffer *buffer);
void buffer_error(StreamBuffer *buffer, StreamErrors error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding unit test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, right now these are tested with every test but pipe ones, but in future I guess I have to choose something.

}
}

static int buffer_read_function(void *user_data, uint8_t *buf, int buf_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding unit test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

@MikeIndiaAlpha
Copy link
Contributor Author

MikeIndiaAlpha commented Jul 19, 2022

Well Done.

:-)

Is loadInputBuffer() planned to be removed in future PR?

I first want to change it into thread (goroutine?) to have synchronisation test. Then it could be removed, or alternatively it could be used to run a few tests of the demuxer kind with the StreamBuffer input?

How do you run tests?

The version that we have right now runs all tests that aren't using pipes through loadInputBuffer. I don't think it should be left this way, more like development help (having various tests and demuxers allowed me to understood requirements of the demuxer wrt to seek() better)

Alternative input system for LPMS. The idea is to allow using of
FFmpeg demultiplexers with any input mechanism, such as network
packet delivery. In order to do that we have to provide custom
AVIOContext with custom read() and seek() operations. This also
means that synchronization is needed because we will have to
perform input and transcoding in different threads (for example
demux - which is a part of transcode - may want more data which
is not available yet, then transcode should block until input
thread delivers more data)
@MikeIndiaAlpha MikeIndiaAlpha force-pushed the ma_stream_buffer branch 5 times, most recently from 13446f1 to c76aec4 Compare July 25, 2022 14:24
@MikeIndiaAlpha
Copy link
Contributor Author

MikeIndiaAlpha commented Jul 25, 2022

@AlexKordic - this is current version of the LL input/output. It works by having circular buffer on the input side (like we discussed) and packet queue on the output side. ffmpeg.go was modified to use LL input/output with all the tests. Namely, input file is loaded into stream buffer before transcode() is called, and output data is picked from the queue after transcode() ends.

First commit works with all the tests. Alas, second commit fails some. This is primarily because mp4 format muxer requires seek() (to update file size or some such in the header after all write operations are completed), and I don't think we want this because it would kill Low Latency (to allow seek and write we'd have to refrain from sending data until the very end, which is exactly what no-LL version does now).

Apart from that, TestTranscoder_API_AlternatingTimestamps test fails for some reason not entirely clear for me (yet). From what I managed to see, .md5 muxer fails to send any data to custom I/O.

But if you replace go procedures loadInputBuffer/storeOutputQueue with threads receiving/sending the data over the network, Transcoder should work with LL. Alternatively, when I'll be back I want to replace these procedures with file I/O threads myself, to make sure that synchronisation is correct.

Output side counterpart to the StreamBuffer. It is not exactly the
same solution, because input and output characteristics are quite
different (for one, on the input side stream is just a series of
bytes, whereas on the output side we are able to tell packets
apart, assign timestamps, packet types, etc).

There is simple Golang code for writing the data into the files,
so that some tests will pass with this code. Note that the big
problem with the queue output is that some muxers (for example
mp4) need to be able to seek() in output, and we don't allow that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants