Message ID | 20201207100845.17520-1-jeebjp@gmail.com |
---|---|
Headers | show |
Series | FIFO meta muxer related improvements | expand |
On Mon, Dec 7, 2020 at 12:08 PM Jan Ekström <jeebjp@gmail.com> wrote: > > The primary parts of this are patches 1,4,5. 2 and 3 were just noticed when > poking at the recovery timestamp logic, where the stream-time comparison logic > seemed somewhat weird (such as comparing the pts to last_recovery_ts only if > last_recovery_ts == AV_NOPTS_VALUE). If they seem incorrectly understood, > they can be left out. > > As for the rest of the patches, they address the following issues: > 1. Even if the header and the first packet get written out, the muxer might > still fail at writing to a server when it actually decides to do I/O. > Currently (for example with the MP4 muxer) this leads to a constant retry > loop as each recovery is actually "successful". This patch makes sure that > even if a recovery is "successful", the following recovery will only happen > after the configured time, easing the load on any receiving server. > 4. In case of avformat_write_header failing, the fifo muxer until now would not > close the avformat context and its underlying I/O. This is now added, so > that file sockets do not keep creeping up. > 5. Unset a configured codec_tag if it is not supported by the underlying muxer, > as the API client has no visibility regarding whether a codec tag is > acceptable in cases of meta-muxers such as fifo. > > Jan > > Bernard Boulay (1): > avformat/fifo: always wait recovery_wait_time between recoveries > > Jan Ekström (4): > avformat/fifo: fix handling of stream-time non-NOPTS recovery > avformat/fifo: cause immediate stream-time recovery if time went > backwards > avformat/fifo: close IO in case header writing fails > avformat/fifo: unset codec tag if unsupported by underlying muxer > > libavformat/fifo.c | 77 ++++++++++++++++++++++++++++++---------------- > 1 file changed, 51 insertions(+), 26 deletions(-) > Ping for this patch set. Jan
On Mon, Jan 11, 2021 at 4:46 PM Jan Ekström <jeebjp@gmail.com> wrote: > > On Mon, Dec 7, 2020 at 12:08 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > > The primary parts of this are patches 1,4,5. 2 and 3 were just noticed when > > poking at the recovery timestamp logic, where the stream-time comparison logic > > seemed somewhat weird (such as comparing the pts to last_recovery_ts only if > > last_recovery_ts == AV_NOPTS_VALUE). If they seem incorrectly understood, > > they can be left out. > > > > As for the rest of the patches, they address the following issues: > > 1. Even if the header and the first packet get written out, the muxer might > > still fail at writing to a server when it actually decides to do I/O. > > Currently (for example with the MP4 muxer) this leads to a constant retry > > loop as each recovery is actually "successful". This patch makes sure that > > even if a recovery is "successful", the following recovery will only happen > > after the configured time, easing the load on any receiving server. > > 4. In case of avformat_write_header failing, the fifo muxer until now would not > > close the avformat context and its underlying I/O. This is now added, so > > that file sockets do not keep creeping up. > > 5. Unset a configured codec_tag if it is not supported by the underlying muxer, > > as the API client has no visibility regarding whether a codec tag is > > acceptable in cases of meta-muxers such as fifo. > > > > Jan > > > > Bernard Boulay (1): > > avformat/fifo: always wait recovery_wait_time between recoveries > > > > Jan Ekström (4): > > avformat/fifo: fix handling of stream-time non-NOPTS recovery > > avformat/fifo: cause immediate stream-time recovery if time went > > backwards > > avformat/fifo: close IO in case header writing fails > > avformat/fifo: unset codec tag if unsupported by underlying muxer > > > > libavformat/fifo.c | 77 ++++++++++++++++++++++++++++++---------------- > > 1 file changed, 51 insertions(+), 26 deletions(-) > > > > Ping for this patch set. Ping for this patch set. Jan
On Mon, Jan 25, 2021 at 10:01 AM Jan Ekström <jeebjp@gmail.com> wrote: > > On Mon, Jan 11, 2021 at 4:46 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > > On Mon, Dec 7, 2020 at 12:08 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > > > > The primary parts of this are patches 1,4,5. 2 and 3 were just noticed when > > > poking at the recovery timestamp logic, where the stream-time comparison logic > > > seemed somewhat weird (such as comparing the pts to last_recovery_ts only if > > > last_recovery_ts == AV_NOPTS_VALUE). If they seem incorrectly understood, > > > they can be left out. > > > > > > As for the rest of the patches, they address the following issues: > > > 1. Even if the header and the first packet get written out, the muxer might > > > still fail at writing to a server when it actually decides to do I/O. > > > Currently (for example with the MP4 muxer) this leads to a constant retry > > > loop as each recovery is actually "successful". This patch makes sure that > > > even if a recovery is "successful", the following recovery will only happen > > > after the configured time, easing the load on any receiving server. > > > 4. In case of avformat_write_header failing, the fifo muxer until now would not > > > close the avformat context and its underlying I/O. This is now added, so > > > that file sockets do not keep creeping up. > > > 5. Unset a configured codec_tag if it is not supported by the underlying muxer, > > > as the API client has no visibility regarding whether a codec tag is > > > acceptable in cases of meta-muxers such as fifo. > > > > > > Jan > > > > > > Bernard Boulay (1): > > > avformat/fifo: always wait recovery_wait_time between recoveries > > > > > > Jan Ekström (4): > > > avformat/fifo: fix handling of stream-time non-NOPTS recovery > > > avformat/fifo: cause immediate stream-time recovery if time went > > > backwards > > > avformat/fifo: close IO in case header writing fails > > > avformat/fifo: unset codec tag if unsupported by underlying muxer > > > > > > libavformat/fifo.c | 77 ++++++++++++++++++++++++++++++---------------- > > > 1 file changed, 51 insertions(+), 26 deletions(-) > > > > > > > Ping for this patch set. > > Ping for this patch set. As there were no comments... I think that while this is a behavioral change (you no longer get an immediate retry in case you have recovery_wait_time set with the first attempt): 1. the current way of doing recovery waits can cause DoS with muxers such as movenc - which can return success initially (and thus the retry counter is reset), but then fail to actually write actual data out 2. I do not think we can ask from the muxer whether it actually sent data out through the I/O or not. Thus, just making the fifo meta muxer always wait for the recovery_wait_time makes sense. Additionally, there are some bug fixes that I found, such as the forgotten closing of I/O, or unsetting the codec tag if it is not supported by the actual underlying format (since the API client has no visibility of the underlying format). Jan