[FFmpeg-devel] [PATCH v5] Add SUP/PGS subtitle demuxer

[wm4]

On Mon, 1 Sep 2014 08:34:52 +0000 (UTC)
Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:

> wm4 <nfxjfg <at> googlemail.com> writes:
> 
> > +    avio_rb32(s->pb); /* discard DTS (usually 0, and useless) */
> 
> Would it be an option to set it dts to AV_NOPTS_VALUE 
> if this value is 0 but use the value otherwise?
> Or is there no advantage?

Probably, see discussion with Petri.

> > +        full_packet_size = AV_RB16(buf + 10 + 1) + 10 + 3;
> > +        if (buf_size < full_packet_size)
> > +            break;
> 
> I don't know if it is worth the effort, but you could 
> check for the first byte as well:
> if (buf[10] != 0x80 || (buf[10] | 3) != 17)
>     break;
> With this change, you could increase the scores, reduce 
> the number of maximum packets in the loop and maybe 
> speed up probing iiuc.

the 10th byte is the segment type field. Are you sure this check covers
all valid types? What if the format is extended later with more types?

> > +    if (nb_packets < 4)
> > +        return AVPROBE_SCORE_RETRY;
> 
> If you find three consecutive packets on the 
> start of the file, at least EXTENSION is 
> appropriate imo even without checking the 
> content, possibly more.

I can do that, though then I'd return AVPROBE_SCORE_RETRY + 1. AFAIK
this is the lowest score that means "safe detection", and it possibly
gives room for other formats in case of misdetection. EXTENSION is
pretty high.

If I change this, I'd have to adjust the following checks too so that
it makes sense. Suggestions?