[FFmpeg-devel] camera files (no name)

[Vitor Sessak]

nicolas martin wrote:
> 
> Le 09-02-06 ? 16:52, Michael Niedermayer a ?crit :
> 
>> On Fri, Feb 06, 2009 at 04:26:02PM -0500, nicolas martin wrote:
>>>
>>> Le 09-02-06 ? 14:06, Michael Niedermayer a ?crit :
>>>
>>>> On Fri, Feb 06, 2009 at 01:43:27PM -0500, nicolas martin wrote:
>>>>>
>>>>> Le 09-02-06 ? 12:20, Michael Niedermayer a ?crit :
>>>>>
>>>>>> On Fri, Feb 06, 2009 at 08:14:30AM -0500, nicolas martin wrote:
>>>>>>>
>>>>>>>> On Wed, Feb 04, 2009 at 01:23:16PM -0500, nicolas martin wrote:
>>>>>>>>>
>>>>>>>>>> On Tue, Feb 03, 2009 at 04:27:51PM -0500, nicolas martin wrote:
>>>>>>>>>>> Attached is a patch to support the nc4600 camera files.
>>>>>>>>>>>
>>>>>>>>>>> Thanks in advance for reviewing and replying.
>>>>>>>>>>
>>>>>>>>>> Changelog and docs update is missing.
>>>>>>>>>
>>>>>>>>> Doc update and changelog since when ?
>>>>>>>>
>>>>>>>> If you add a new demuxer, you should add an entry for it to the
>>>>>>>> changelog and the format support list in doc/general.texi.
>>>>>>>>
>>>>>>>
>>>>>>> In reply to what Peter Ross wrote, I can't find its mail though
>>>>>>>
>>>>>>>>>> + if (size <= 0) {
>>>>>>>>>> + av_log(s, AV_LOG_DEBUG, "Wrong size : %d\n", size);
>>>>>>>>> While size may be zero, it will never be less then zero, thus the
>>>>>>>>> debug
>>>>>>>>> statement is pointless.
>>>>>>> The size can't be less than zero, I changed it.
>>>>>>> However if the size is zero, then we have a problem, so the debug
>>>>>>> statement
>>>>>>> is meaningful.
>>>>>>
>>>>>> [...]
>>>>>>> +static int nc_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>>>>> +{
>>>>>>> +    int size;
>>>>>>> +
>>>>>>> +    uint32_t state=-1;
>>>>>>> +    while(!url_feof(s->pb) && state != NC_VIDEO_FLAG)
>>>>>>> +        state = (state<<8) + get_byte(s->pb);
>>>>>>> +
>>>>>>> +    get_byte(s->pb);
>>>>>>> +    size = get_le16(s->pb);
>>>>>>> +    url_fskip(s->pb, 9);
>>>>>>> +
>>>>>>
>>>>>>> +    if (size == 0) {
>>>>>>> +        av_log(s, AV_LOG_DEBUG, "Wrong size : %d\n", size);
>>>>>>> +        return AVERROR_INVALIDDATA;
>>>>>>> +    }
>>>>>>
>>>>>> what is the problem with not returning an error in this case?
>>>>>
>>>>> Well I think that the camera is not supposed to send packet with 
>>>>> size 0.
>>>>> If
>>>>> it happens, it probably means the data got corrupted, or an error
>>>>> occured.
>>>>> In any case, I don't think you can do anything besides returning an 
>>>>> error
>>>>> ... maybe try to skip data until the next header ??
>>>>> As I don't think it is supposed to happen, I choose to treat it like
>>>>> this.
>>>>> What do you think ??
>>>>
>>>> id print a warning and continue, the code should be able to find and
>>>> continue
>>>> at the next valid frame
>>>
>>> Ok so I changed it so that while (size==0) it loops until it finds a 
>>> valid
>>> size.
>>> Though i think it should never happen, if it happens, better pray 
>>> that it's
>>> a corrupted data,
>>> my guess is that it will loop forever ...
>>> Should I add a count to break if it repeateadly gives a null size ? 
>>> or is
>>> the solution I brought enough ?
>>
>> it should fail at EOF to prevent an infinite loop
>> and it might make sense to retun EAGAIN instead of looping, this also
>> might be simpler
> 
> What about something like that ?

I was going to apply, but it failed to decode the first sample you sent 
(ftp://upload.mplayerhq.hu/incoming/nc_camera/nc_sample.avi), giving 
repeatedly the message:

  [nc @ 0x88e3b60]Next paquet size is zero

The second sample plays fine, but I'm not sure if in the correct speed. 
Also, since you'll have to do another version anyway, one nit:

> +
> +    if (size == 0) {
> +        av_log(s, AV_LOG_DEBUG, "Next paquet size is zero\n");

Next _packet_...

-Vitor