[FFmpeg-devel] [PATCH] avformat/mpegts: use get_packet_size in mpegts_resync for determining raw_packet_size

[lance.lmwang at gmail.com]

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. 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 (ret < 0)
> > +                return ret;
> > +            new_packet_size = get_packet_size(s);
> > +            if (new_packet_size > 0 && new_packet_size != ts->raw_packet_size) {
> > +                av_log(ts->stream, AV_LOG_WARNING, "changing packet size to %d\n", new_packet_size);
> > +                ts->raw_packet_size = new_packet_size;
> > +            }
> > +            avio_seek(pb, pos, SEEK_SET);
> >             return 0;
> >         }
> >     }
> > -- 
> > 2.26.1
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang