[FFmpeg-devel] NC camera patch
Vitor Sessak
vitor1001
Sat Jan 10 01:18:03 CET 2009
nicolas martin wrote:
>
>> nicolas martin wrote:
>>
>>> Hi all,
>>>
>>> I finally made some code to read the camera feed sent by NC46** types
>>> cameras.
>>>
>>> Thanks for reading and sharing your thoughts !
>>
>> The format looks quite simple and your code quite complicated.
>> Maybe I missed some subtlety in the format.
>> So let me first describe the format as I understand it:
>> Each frame is composed of:
>> - A header (16 bytes)
>> * 4 bytes : a flag (eg: 0x1A5 (big-endian))
>> * 1 byte : unknown/unused
>> * 2 bytes : data_size (only when flag == 0x1A5)
>> * 9 bytes : unknown/unused
>> - MPEG4 data frame (data_size bytes)
>> Please correct me if there's something wrong here.
>>
>> If my description is right, you could write your frame reader
>> function trivially, with something like that:
>>
>> flag = get_be32(pb);
>> get_byte(pb);
>> size = get_le16(pb);
>> url_fskip(pb, 9);
>> if (flag != 0x1A5)
>> /* error handling */;
>> av_new_packet(pkt, size)
>> get_buffer(pb, pkt->data, size);
>>
>> You need some more error checking, etc...
>> But basically, this code should be enough.
>>
>> BTW: you should upload a sample file like described in [1] to allow
>> other developers to test your code.
>>
> Ok I uploaded a sample in nc_camera
>
>> Oh, and your nc_read_header() seems to contain code which is
>> totally unrelated to your format (MJPEG, DIRAC, etc...).
>> And you don't need to use s->iformat->value if it's hardcoded
>> to MPEG4.
>
> I joined the version I modified using your tips.
> I still don't know what to put in nc_read_header() by the way
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.
> +static int nc_read_header(AVFormatContext *s,
> + AVFormatParameters *ap)
Nit: you don't need a line break here.
> +{
> + AVStream *st;
> + st = av_new_stream(s, 0);
Nit: You can merge this as
AVStream *st = av_new_stream(s, 0);
> + if (!st)
> + return AVERROR(ENOMEM);
> +
> + st->codec->codec_type = CODEC_TYPE_VIDEO;
> + st->codec->codec_id = s->iformat->value;
Here you can do as Aurelien sugested and set directly codec_id to
CODEC_ID_MPEG4...
> + /* for MJPEG, specify frame rate */
> + /* for MPEG-4 specify it, too (most MPEG-4 streams do not have the fixed_vop_rate set ...)*/
> + if (ap->time_base.num) {
> + av_set_pts_info(st, 64, ap->time_base.num, ap->time_base.den);
> + } else if ( st->codec->codec_id == CODEC_ID_MJPEG ||
> + st->codec->codec_id == CODEC_ID_MPEG4 ||
> + st->codec->codec_id == CODEC_ID_DIRAC ||
> + st->codec->codec_id == CODEC_ID_H264) {
> + av_set_pts_info(st, 64, 1, 25);
> + }
...and them this block is the same as just
av_set_pts_info(st, 64, 1, 25);
> + return 0;
> +}
> +
> +static int nc_read_partial_packet(AVFormatContext *s, AVPacket *pkt)
I don't think the "partial" in the name is relevant anymore
> +{
> + uint32_t flag;
> + int size, ret;
> +
> + flag = get_le32(s->pb);
Nit:
int size, ret;
uint32_t flag = get_le32(s->pb);
[...]
> +AVInputFormat nc_demuxer = {
> + "nc",
> + NULL_IF_CONFIG_SMALL("NC camera feed format"),
> + NULL,
> + nc_probe,
> + nc_read_header,
> + nc_read_partial_packet,
> + .extensions = "v",
> + .value = CODEC_ID_MPEG4,
As Aurelien said, if you set codec_id directly to CODEC_ID_MPEG4 in
nc_read_header(), this last line is unnecessary.
-Vitor
More information about the ffmpeg-devel
mailing list