[FFmpeg-devel] [PATCH 2/2] ADP demuxer

[James Almer]

On 23/04/13 7:54 PM, Michael Niedermayer wrote:
> On Sat, Apr 20, 2013 at 08:12:24PM -0300, James Almer wrote:
>> +static int adp_probe(AVProbeData *p)
>> +{
>> +    if (p->buf[0] == p->buf[2] && p->buf[1] == p->buf[3]) {
>> +        if (p->buf_size < 260)
>> +            return AVPROBE_SCORE_MAX / 4 - 1;
> 
> return 0 or 1 here IMHO
> just a single frame is very odd and doesnt look like a correct file

Frames are 32 bytes long. I arbitrarily chose 256-259 when checking for another 
valid frame to avoid tiertexseq files being misdetected as adp (The first 256 bytes 
are all 0 on tiertexseq).
If buf_size is < 260 and the probe for tiertexseq succeeds, then the probe for adp 
will succeed as well. Making adp return a lower score than tiertexseq in that case 
will make sure tiertexseq is chosen.

I'll make it return 0 if that's preferred, though.

>> +    if (s->pb->seekable)
>> +        st->duration          = avio_size(s->pb) / 32 * 28;
> 
> Is this check needed ?
> avio_size() should harmlessly fail if the protocal cant seek

Many other demuxers check for pb->seekable before calling avio_seek() or avio_size(),
so i figured it was the proper way to handle this.
Personally, I'd like to keep it this way.

>> +AVInputFormat ff_adp_demuxer = {
>> +    .name           = "adp",
>> +    .long_name      = NULL_IF_CONFIG_SMALL("ADP"),
>> +    .read_probe     = adp_probe,
>> +    .read_header    = adp_read_header,
>> +    .read_packet    = adp_read_packet,
>> +    .extensions     = "adp,dtk",
>> +};
> 
> does seeking work with this demuxer ?
> (also see AVFMT_GENERIC_INDEX)

Doesn't look like, even after adding that flag "ffmpeg -ss 1 -i sample.adp" warns 
that it couldn't seek.

> No more comments from me, rest looks good, feel free to ask for a
> merge once all dependancies are ready too

Ok, thanks.