[FFmpeg-devel] NC camera patch
Michael Niedermayer
michaelni
Sat Jan 10 14:49:35 CET 2009
On Fri, Jan 09, 2009 at 10:39:19PM -0500, 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.
>>>>>
>>>>> 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.
>
> 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.
>
>>
>>
>>> +static int nc_read_header(AVFormatContext *s,
>>> + AVFormatParameters *ap)
>>
>> Nit: you don't need a line break here.
>
> Changed.
>
>>
>>
>>> +{
>>> + AVStream *st;
>>> + st = av_new_stream(s, 0);
>>
>> Nit: You can merge this as
>>
>> AVStream *st = av_new_stream(s, 0);
>
> Changed.
>
>>
>>
>>> + 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...
>
> Changed.
>
>>
>>
>>> + /* 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);
>
> Changed.
>
>>
>>
>>> + return 0;
>>> +}
>>> +
>>> +static int nc_read_partial_packet(AVFormatContext *s, AVPacket *pkt)
>>
>> I don't think the "partial" in the name is relevant anymore
>
> Changed.
>
>>
>>
>>> +{
>>> + uint32_t flag;
>>> + int size, ret;
>>> +
>>> + flag = get_le32(s->pb);
>>
>> Nit:
>>
>> int size, ret;
>> uint32_t flag = get_le32(s->pb);
>>
>> [...]
>
> Changed.
>
>>
>>
>>> +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.
>
> Changed.
>
> The new patch is attached.
[...]
> +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
[...]
> +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
[...]
> + if (av_new_packet(pkt, size) < 0)
> + return AVERROR(EIO);
> +
> + pkt->pos = url_ftell(s->pb);
> + pkt->stream_index = 0;
> +
> + ret = get_buffer(s->pb, pkt->data, size);
> + if (ret<=0) {
> + av_free_packet(pkt);
> + return AVERROR(EIO);
> + }
this can be simplified with av_get_packet()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- 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/20090110/c56f5513/attachment.pgp>
More information about the ffmpeg-devel
mailing list