[FFmpeg-devel] NC4600 camera code

[Diego Biurrun]

On Sat, Dec 13, 2008 at 10:51:21AM -0500, nicolas martin wrote:
>> On Sat, Dec 13, 2008 at 10:01:38AM -0500, nicolas martin wrote:
>>>> On Sat, Dec 13, 2008 at 08:52:38AM -0500, nicolas martin wrote:
>>>>
>>>> If you want to see the patch reviewed and possibly included...
>>> The patch is attached
>>>
>>> --- libavformat/avio.h	(revision 16101)
>>> +++ libavformat/avio.h	(working copy)
>>> @@ -172,6 +172,12 @@
>>> +#ifdef CONFIG_NC4600_DEMUXER
>>> +    int copied;
>>> +    int paquet_size;
>>
>> 'paquet' is not an English word.  Did you mean packet?
> Yes, sorry for the bad traduction, it is fixed.
>>
>>
>>> --- libavformat/aviobuf.c	(revision 16101)
>>> +++ libavformat/aviobuf.c	(working copy)
>>> @@ -417,6 +422,73 @@
>>>
>>> +#ifdef CONFIG_NC4600_DEMUXER
>>> +//this gives the size of the next real data paquet based on the 16  
>>> bytes header of the paquet (i.e those 16 bytes must be trimmed from  
>>> the mpeg4 stream)
>>
>> Please try to keep lines below 80 characters.
> Fixed
>>
>>
>>> +int nc4600_ident_packet(unsigned char *p) {
>>
>> Put the opening brace on the next line.
> Fixed
>
> Last patch attached.
>
> --- libavformat/raw.c	(revision 16101)
> +++ libavformat/raw.c	(working copy)
> @@ -914,6 +957,20 @@
> 
> +#ifdef CONFIG_NC4600_DEMUXER
> +AVInputFormat nc4600_demuxer = {
> +    "nc4600",
> +    NULL_IF_CONFIG_SMALL("special raw MPEG-4 video format"),
> +    0,
> +    nc4600_mpeg4video_probe, /** probing for MPEG-4 data */

IMO this comment is pointless.

> --- libavformat/avio.h	(revision 16101)
> +++ libavformat/avio.h	(working copy)
> @@ -273,6 +279,17 @@
> 
> +#ifdef CONFIG_NC4600_DEMUXER

This #ifdef is unnecessary.

> +/**
> + * Reads size bytes from ByteIOContext into buf.
> + * This reads at most 1 packet. If that is not enough fewer bytes will be
> + * returned.
> + * Handle special nc4600 cameras packet with 16 bytes header

Handle special nc4600 camera packets with 16 bytes header.

> --- libavformat/aviobuf.c	(revision 16101)
> +++ libavformat/aviobuf.c	(working copy)
> @@ -417,6 +422,76 @@
> 
> +#ifdef CONFIG_NC4600_DEMUXER
> +//this gives the size of the next real data packet based on the 16 bytes
> +//header of the packet (those 16 bytes must be trimmed from the mpeg4 stream)

MPEG-4

> +#ifndef MIN
> +#define MIN(x,y) ((x)<(y)?(x):(y))
> +#endif

Such a macro already exists, it is called FFMIN.

> +int nc4600_get_partial_buffer(ByteIOContext *s, unsigned char *buf, int size)
> +{
> +    int len,t;

A space after the comma will make things more readable..

Diego

P.S.: Your mails would be more readable if you would leave an empty line
between the quoted text and your answer.