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

[Michael Niedermayer]

On Tue, Apr 23, 2013 at 11:28:26PM -0300, James Almer wrote:
> 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 

in that case you could test all the headers within 260+ bytes instead
of just 2


> 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.

i think 1 is better
returning 0 for short adp files doesnt feel correct


> 
> >> +    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.

whichever way you prefer


> 
> >> +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.

hmm
probably would make sense to compare with a other simple demuxer that
can seek


> 
> > No more comments from me, rest looks good, feel free to ask for a
> > merge once all dependancies are ready too
> 
> Ok, thanks.
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130424/8ef97d21/attachment.asc>