| 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
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(-)