[Ffmpeg-devel] RTP patches & RFC

[Ryan Martell]

Here's another one; all of that stuff should be sorted out (I hope!)



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:
> [...]
>>> [...]
>>>
>>
>> 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)

Fixed.

> [...]
>> @@ -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

Fixed.

> [...]
>>      int i;
>> @@ -153,12 +158,22 @@
>>         see if we can handle this kind of payload */
>>      get_word_sep(buf, sizeof(buf), "/", &p);
>>      if (payload_type >= RTP_PT_PRIVATE) {
>> -        /* We are in dynmaic payload type case ... search into  
>> AVRtpDynamicPayloadTypes[] */
>> -        for (i = 0; AVRtpDynamicPayloadTypes[i].codec_id !=  
>> CODEC_ID_NONE; ++i)
>> -            if (!strcmp(buf, AVRtpDynamicPayloadTypes 
>> [i].enc_name) && (codec->codec_type == AVRtpDynamicPayloadTypes 
>> [i].codec_type)) {
>> -                codec->codec_id = AVRtpDynamicPayloadTypes 
>> [i].codec_id;
>> -                break;
>> -            }
>> +        {
>> +            RTPDynamicProtocolHandler *handler=  
>> RTPFirstDynamicPayloadHandler;
>> +            while(handler)
>> +            {
>> +                if (!strcmp(buf, handler->enc_name) && (codec- 
>> >codec_type == handler->codec_type)) {
>> +                    codec->codec_id = handler->codec_id;
>> +                    rtsp_st->dynamic_handler= handler;
>> +                    if(handler->protocol_new_extradata_proc)
>> +                    {
>> +                        rtsp_st->dynamic_protocol_data= handler- 
>> >protocol_new_extradata_proc();
>> +                    }
>> +                    break;
>> +                }
>> +                handler= handler->next;
>> +            }
>> +        }
>>      } else {
>
> the first and last {} are unneeded
> the other {} dont match the placement in this file

Fixed.

> [...]
>> @@ -451,7 +466,15 @@
>>                  st = s->streams[i];
>>                  rtsp_st = st->priv_data;
>>                  if (rtsp_st->sdp_payload_type == payload_type) {
>> -                    sdp_parse_fmtp(st, p);
>> +                    if(rtsp_st->dynamic_handler && rtsp_st- 
>> >dynamic_handler->protocol_handle_sdp_a_line_proc)
>> +                    {
>> +                        if(!rtsp_st->dynamic_handler- 
>> >protocol_handle_sdp_a_line_proc(st, rtsp_st- 
>> >dynamic_protocol_data, buf))
>> +                        {
>> +                            sdp_parse_fmtp(st, p);
>> +                        }
>> +                    } else {
>> +                        sdp_parse_fmtp(st, p);
>> +                    }
>>                  }
>>              }
>>          }
>
> same {} problem

Fixed.

> [...]
>> -        if (!rtsp_st->rtp_ctx) {
>> -            err = AVERROR_NOMEM;
>> -            goto fail;
>> -        }
>> +        if(rtsp_st->rtp_ctx)
>> +        {
>> +            if(rtsp_st->dynamic_handler)
>> +            {
>> +                rtsp_st->rtp_ctx->dynamic_protocol_data= rtsp_st- 
>> >dynamic_protocol_data;
>> +                rtsp_st->rtp_ctx->dynamic_packet_handler= rtsp_st- 
>> >dynamic_handler->protocol_handle_packet_proc;
>> +            }
>> +        } else {
>> +            err = AVERROR_NOMEM;
>> +            goto fail;
>> +        }
>>      }
>
> here too
>
Fixed & made smaller patcfh (left in the if (!r, and handled as an else)

>>
>>      /* use callback if available to extend setup */
>> @@ -1323,7 +1355,14 @@
>>          if (!st)
>>              s->ctx_flags |= AVFMTCTX_NOHEADER;
>>          rtsp_st->rtp_ctx = rtp_parse_open(s, st, rtsp_st- 
>> >sdp_payload_type, &rtsp_st->rtp_payload_data);
>> -        if (!rtsp_st->rtp_ctx) {
>> +        if(rtsp_st->rtp_ctx)
>> +        {
>> +            if(rtsp_st->dynamic_handler)
>> +            {
>> +                rtsp_st->rtp_ctx->dynamic_protocol_data= rtsp_st- 
>> >dynamic_protocol_data;
>> +                rtsp_st->rtp_ctx->dynamic_packet_handler= rtsp_st- 
>> >dynamic_handler->protocol_handle_packet_proc;
>> +            }
>> +        } else {
>>              err = AVERROR_NOMEM;
>
> 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 ...)

Hope this is a winner; I really want to get the rest of this stuff in...

-Ryan
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061025/9b1ab40d/attachment.txt>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtp_internal.h
Type: application/octet-stream
Size: 3517 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061025/9b1ab40d/attachment.obj>
-------------- next part --------------