[FFmpeg-devel] NC camera patch
nicolas martin
elvadrias
Mon Jan 12 22:56:13 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 ..
Ok I'll use this instead in read_packet!
>
>
>
>
>>
>> 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?
No it does not work without AVSTREAM_PARSE_FULL.
>
>
>
> [...]
>
>> +#define NC_VIDEO_FLAG 0xA5010000
>
> i really think this is supposed to be 0x1A5 and read as big endian
Changed.
>
>
>
> [...]
>> +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 ...
Changed it for
av_set_pts_info(st, 64, ap->time_base.num, ap->time_base.den);
I suppose this is the standard way for MPEG4 streams
>
>
>
>> +
>> + 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?
Not really, I suppose they have a meaning when you enable audio, or
other features of the camera (NTP synchronisation, etc...).
>
>
>
> [...]
> --
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
New patch attached.
Nicolas
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090112/5b33e885/attachment.txt>
-------------- next part --------------
More information about the ffmpeg-devel
mailing list