[Ffmpeg-devel] RTP patches & RFC

[Ryan Martell]

On Oct 23, 2006, at 7:15 PM, Michael Niedermayer wrote:

> Hi
>
> On Mon, Oct 23, 2006 at 06:38:13PM -0500, Ryan Martell wrote:
> [...]
>>> [...]
>>>
>>>
>>>> @@ -373,7 +366,13 @@
>>>>     uint32_t timestamp;
>>>>
>>>>     if (!buf) {
>>>> -        /* return the next packets, if any */
>>>> +        if(s->st && s->dynamic_packet_handler)
>>>> +        {
>>>> +            return s->dynamic_packet_handler(s, pkt, 0, NULL, 0);
>>>> +        }
>>>> +        else
>>>> +        {
>>>> +            /* return the next packets, if any */
>>>>         if (s->read_buf_index >= s->read_buf_size)
>>>>             return -1;
>>>>         ret = mpegts_parse_packet(s->ts, pkt, s->buf + s-
>>>>> read_buf_index,
>>>> @@ -385,6 +384,7 @@
>>>>             return 1;
>>>>         else
>>>>             return 0;
>>>> +        }
>>>>     }
>>>
>>> this should rather look like:
>>>
>>>     if (!buf) {
>>>         /* return the next packets, if any */
>>> +      if(s->st && s->dynamic_packet_handler) {
>>> +            return s->dynamic_packet_handler(s, pkt, 0, NULL, 0);
>>> +      } else {
>>>         if (s->read_buf_index >= s->read_buf_size)
>>>             return -1;
>>>         ret = mpegts_parse_packet(s->ts, pkt, s->buf + s-
>>>> read_buf_index,
>>> @@ -385,6 +384,7 @@
>>>             return 1;
>>>         else
>>>             return 0;
>>> +      }
>>>     }
>>>
>>> * the /* return the next packets, if any */ was reindented
>>> * the {} placement doesnt match the rest of the file
>>> * the indention level isnt optimal unless you plan to send a
>>>  seperate patch to fix the indention after this one
>>
>> I didn't want to touch the indentation initially.  I think the
>> included is what you're looking for.
>
> no, i meant the new code should be indented between the levels of the
> existing code like in my example above (IMHO that makes it more  
> readable
> then having 2 blocks which should be on different indention levels  
> on the
> same level)
> or alternatively your original
> change but with a _seperate_ patch which only indents the then wrongly
> idented block by +4 (seperating them helps keeping svn log / svn
> history easily readable)

I gotcha;  I thought you meant that you wanted me to reindent (once I  
fixed my stuff to be the same as the rest of the file)...

> [...]
>> @@ -457,8 +457,13 @@
>>              memcpy(pkt->data, buf, len);
>>              break;
>>          default:
>> -            av_new_packet(pkt, len);
>> -            memcpy(pkt->data, buf, len);
>> +            if(s->dynamic_packet_handler)
>> +            {
>> +                return s->dynamic_packet_handler(s, pkt,  
>> timestamp, buf, len);
>> +            } else {
>> +                av_new_packet(pkt, len);
>> +                memcpy(pkt->data, buf, len);
>> +            }
>>              break;
>
> the {} placement is inconsistant here IMHO
>
>
> [...]
>>      } else {
>
> the first and last {} are unneeded
> the other {} dont match the placement in this file
>
>
> [...]
> same {} problem
>
>
> [...]
> here too
>
> and here
>
> but note, i wont reject this because of the {} placement if you  
> disslike
> it (fixing it myself would just need ~5min or so ...)
>
> [...]
> -- 
> Michael     GnuPG fingerprint:  
> 9FF2128B147EF6730BADF133611EC787040B0FAB

If you could fix it, that would be fine.  I changed my development  
environment to use spaces instead of tabs, so it should sort itself  
out now.  Those were all my replace tabs with spaces, and the spaces  
I was replacing it with must have been off by one.  More incentive to  
get clean-diff working!

And on my own code, I'm using indent per dev specs...

I've grown used to having {} on separate lines; I'll just have to  
wean myself off that for FFMPEG!

Thanks!
-R