[FFmpeg-devel] NC camera patch

Michael Niedermayer michaelni
Mon Jan 12 21:05:56 CET 2009


On Mon, Jan 12, 2009 at 11:30:16AM -0500, nicolas martin wrote:
>> 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 ?

sorry, ive misread your code (i thought the check was in read_header() where
it would be redundant)

in read_packet its not ideal either though ...

If some fixed 4 byte word occurs at the start of each packet you should
use something like

uint32_t state=-1;
while(!url_feof() && state != 0x01A5)
    state= (state<<8) + get_byte(s->pb);

The advantage is that its more robust. For example in case of some error
in the data above will just continue at the next such header ..



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

Does it work without AVSTREAM_PARSE_FULL or not?


[...]

> +#define NC_VIDEO_FLAG 0xA5010000

i really think this is supposed to be 0x1A5 and read as big endian


[...]
> +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;
> +

> +    av_set_pts_info(st, 64, 1, 25);

is 25 always correct, if there are 30fps files than it is not correct ...


> +
> +    return 0;
> +}
> +

> +static int nc_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
[...]
> +    get_byte(s->pb);
[...]
> +    url_fskip(s->pb, 9);

any idea what these bytes are?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090112/35f75ea6/attachment.pgp>



More information about the ffmpeg-devel mailing list