On Wed, May 27, 2020 at 08:47:54AM +0200, Marton Balint wrote:
>
>
> On Wed, 27 May 2020, lance.lmw...@gmail.com wrote:
>
> > On Tue, May 26, 2020 at 09:52:45PM +0200, Marton Balint wrote:
> > >
> > >
> > > On Wed, 20 May 2020, Marton Balint wrote:
> > >
> > > > The old resync logic had some bugs, for example the packet size could
> > > > stuck
> > > > into 192 bytes, because pos47_full was not updated for every packet,
> > > > and for
> > > > unseekable inputs the resync logic simply skipped some 0x47 sync bytes,
> > > > therefore the calculated distance between sync bytes was a multiple of
> > > > 188
> > > > bytes.
> > > > > AVIO only buffers a single packet (for UDP/mpegts, that usually
> > > means 1316
> > > > bytes), so for every ten consecutive 188-byte MPEGTS packets there was
> > > > always a
> > > > seek failure, and that caused the old code to not find the 188 byte
> > > > pattern
> > > > across 10 consecutive packets.
> > > > > This patch changes the custom logic to the one which is used
> > > when probing to
> > > > determine the packet size. This was already proposed as a FIXME for a
> > > > long
> > > > time...
> > >
> > > Ping, will apply soon.
> > >
> > > Regards,
> > > Marton
> > >
> > > > ---
> > > > libavformat/mpegts.c | 51 ++++++++++----------------------------------
> > > > 1 file changed, 11 insertions(+), 40 deletions(-)
> > > > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > > > index a065c61c40..c6fd3e1cef 100644
> > > > --- a/libavformat/mpegts.c
> > > > +++ b/libavformat/mpegts.c
> > > > @@ -123,10 +123,6 @@ struct MpegTSContext {
> > > > /** raw packet size, including FEC if present */
> > > > int raw_packet_size;
> > > > > - int size_stat[3];
> > > > - int size_stat_count;
> > > > -#define SIZE_STAT_THRESHOLD 10
> > > > -
> > > > int64_t pos47_full;
> > > > > /** if true, all pids are analyzed to find streams */
> > > > @@ -2840,41 +2836,6 @@ static int handle_packet(MpegTSContext *ts,
> > > > const uint8_t *packet, int64_t pos)
> > > > return 0;
> > > > }
> > > > > -static void reanalyze(MpegTSContext *ts) {
> > > > - AVIOContext *pb = ts->stream->pb;
> > > > - int64_t pos = avio_tell(pb);
> > > > - if (pos < 0)
> > > > - return;
> > > > - pos -= ts->pos47_full;
> > > > - if (pos == TS_PACKET_SIZE) {
> > > > - ts->size_stat[0] ++;
> > > > - } else if (pos == TS_DVHS_PACKET_SIZE) {
> > > > - ts->size_stat[1] ++;
> > > > - } else if (pos == TS_FEC_PACKET_SIZE) {
> > > > - ts->size_stat[2] ++;
> > > > - }
> > > > -
> > > > - ts->size_stat_count ++;
> > > > - if (ts->size_stat_count > SIZE_STAT_THRESHOLD) {
> > > > - int newsize = 0;
> > > > - if (ts->size_stat[0] > SIZE_STAT_THRESHOLD) {
> > > > - newsize = TS_PACKET_SIZE;
> > > > - } else if (ts->size_stat[1] > SIZE_STAT_THRESHOLD) {
> > > > - newsize = TS_DVHS_PACKET_SIZE;
> > > > - } else if (ts->size_stat[2] > SIZE_STAT_THRESHOLD) {
> > > > - newsize = TS_FEC_PACKET_SIZE;
> > > > - }
> > > > - if (newsize && newsize != ts->raw_packet_size) {
> > > > - av_log(ts->stream, AV_LOG_WARNING, "changing packet size
> > > > to %d\n", newsize);
> > > > - ts->raw_packet_size = newsize;
> > > > - }
> > > > - ts->size_stat_count = 0;
> > > > - memset(ts->size_stat, 0, sizeof(ts->size_stat));
> > > > - }
> > > > -}
> > > > -
> > > > -/* XXX: try to find a better synchro over several packets (use
> > > > - * get_packet_size() ?) */
> > > > static int mpegts_resync(AVFormatContext *s, int seekback, const
> > > > uint8_t *current_packet)
> > > > {
> > > > MpegTSContext *ts = s->priv_data;
> > > > @@ -2896,8 +2857,18 @@ static int mpegts_resync(AVFormatContext *s, int
> > > > seekback, const uint8_t *curren
> > > > if (avio_feof(pb))
> > > > return AVERROR_EOF;
> > > > if (c == 0x47) {
> > > > + int new_packet_size, ret;
> > > > avio_seek(pb, -1, SEEK_CUR);
> > > > - reanalyze(s->priv_data);
> > > > + pos = avio_tell(pb);
> > > > + ret = ffio_ensure_seekback(pb, PROBE_PACKET_MAX_BUF);
> >
> > PROBE_PACKET_MAX_BUF is about 8K, maybe it'll cost too much for the
> > detection with get_packet_size()
> > during the runtime.
>
> But the function uses avio_read_partial which only reads the amount which is
> already in the input buffer, and only continues if the result is
> inconclusive.
>
> > How about to detect the next 3 sync byte with the probe size? it's used by
> > vlc,
> > refer to DetectPacketSize() in modules/demux/mpeg/ts.c
>
> If you check the probing function, it uses PROBE_PACKET_MARGIN (5), which
> means that 6 sync bytres are needed. Not much difference, so I'd rather keep
> things as is.
>
> Practically this only waits for at most one additional UDP packet, and even
> that is not lost, because we ensure the buffer is seekable even for
> non-streamed input.
Thanks for the explanation, I'm fine with the patch.
>
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
--
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
