[FFmpeg-devel] NC camera patch

[nicolas martin]

> nicolas martin wrote:
>>
>>> nicolas martin wrote:
>>>>
>>>>> nicolas martin wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> I finally made some code to read the camera feed sent by NC46**  
>>>>>> types
>>>>>> cameras.
>
> [...]
>
>>>
>>> Just my two cents...
>>>
>>> [...]
>>>
>>>> +/*
>>>> + * NC cameras feed demuxer.
>>>> + * Copyright (c) 2008  Nicolas Martin <martinic at iro.umontreal.ca>,
>>>> Edouard Auvinet
>>>> + *
>>>> + * This file is part of FFmpeg.
>>>> + *
>>>> + * FFmpeg is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later  
>>>> version.
>>>> + *
>>>> + * FFmpeg is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the  
>>>> GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General  
>>>> Public
>>>> + * License along with FFmpeg; if not, write to the Free Software
>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>>> 02110-1301 USA
>>>> + */
>>>> +
>>>> +#include "avformat.h"
>>>> +
>>>> +#define NC_VIDEO_FLAG 0xA5010000
>>>> +
>>>> +static int nc_probe(AVProbeData *probe_packet)
>>>> +{
>>>> +    return
>>>> (AV_RL32(probe_packet->buf)==NC_VIDEO_FLAG?AVPROBE_SCORE_MAX:0);
>>>> +}
>>>> +
>>>
>>> If I understood well the format, the next four bytes should also be
>>> NC_VIDEO_FLAG for valid files. So, its better to check also for  
>>> them to
>>> avoid false positives.
>>
>> Just the first four bytes should be NC_VIDEO_FLAG, the next one is
>> unused and the two following are the size of the next paquet.
>
> Indeed. I should not read code so late ;-) So maybe something like the
> following would be more robust:
>
> static int nc_probe(AVProbeData *probe_packet)
> {
>     int size;
>
>     if (AV_RL32(probe_packet->buf) != NC_VIDEO_FLAG)
>         return 0;
>
>     size = AV_RL16(probe_packet->buf + 5);
>
>     if (size + 20 > probe_packet->buf_size)
>         return 3*AVPROBE_SCORE_MAX/2;
>
>     if (AV_RL32(probe_packet->buf+16+size) == NC_VIDEO_FLAG)
>         return AVPROBE_SCORE_MAX;
>     else
>         return 0;
> }

I included this in my patch.
However, according to what Michael said, and forgive me if I  
misunderstood :

> [...]
>> +static int nc_read_packet(AVFormatContext *s, AVPacket *pkt)
>> +{
>> +    int size, ret;
>> +    uint32_t flag = get_le32(s->pb);
>
>> +    if (flag != NC_VIDEO_FLAG) {
>> +        av_log(NULL, AV_LOG_DEBUG, "wrong flag for nc video  
>> format : %u\n", flag);
>> +        return AVERROR_INVALIDDATA;
>> +    }
>
> this is unneeded, it already was checked in probe


Does it mean, nc_probe is called everytime a new packet is received,  
and before nc_read_packet is called, or
just that we checked it once for all, and suppose it will be true for  
the rest of the video ?

If it is, then maybe nc_probe is not so efficient this way (still more  
robust).
And if it is not, then OK !

>
>
> Also your code gives the warning
>
>> Seems stream 0 codec frame rate differs from container frame rate:  
>> 100.00 (100/1) -> 25.00 (25/1)
>
> Is the frame rate correct? I cannot be seem with your sample (no
> movement)...

Regarding this, what could make the video not be at the correct frame  
rate ? It is supposed to be 25 or 30, I'll have to check.
I will upload another sample with movement so that you can check fps.


> [...]
>> +static int nc_read_header(AVFormatContext *s, AVFormatParameters  
>> *ap)
>> +{
>> +    AVStream *st = av_new_stream(s, 0);
>> +    if (!st)
>> +        return AVERROR(ENOMEM);
>> +
>> +    st->codec->codec_type = CODEC_TYPE_VIDEO;
>> +    st->codec->codec_id = CODEC_ID_MPEG4;
>
>> +    st->need_parsing = AVSTREAM_PARSE_FULL;
>
> Does the format need this? if not it should be removed otherwise it
> can stay

I'm not sure if everything is needing. I'm pretty sure  
CODEC_TYPE_VIDEO is correct and CODEC_ID_MPEG4 also.
I don't know about AVSTREAM_PARSE_FULL, it was suggested by another  
developper.

However, I know that cameras can send audio as well, I just disabled  
it because 1) I don't need it 2) I did'nt work on reverse-engineering  
the headers for it.
I suppose sometime, another developper might work on it (or not), and  
this time CODEC_TYPE_VIDEO should be changed ...


Attached, the new corrected patch.

Thanks

Nicolas

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090112/ad7fa64f/attachment.txt>
-------------- next part --------------


>
>
> -Vitor
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel