[Ffmpeg-devel] RTP patches & RFC

[Ryan Martell]

Golden, maybe?

On Oct 23, 2006, at 4:42 PM, Michael Niedermayer wrote:

> Hi
>
> On Mon, Oct 23, 2006 at 12:34:28PM -0500, Ryan Martell wrote:
> [...]
>
> the patch contains tabs and trailing whitespace these arent
> allowed in svn, a simple search and replace should fix this (or
> clean-diff from ffmpeg-svn run over the patch)

clean-diff doesn't work on my machine, for some reason.  I searched &  
replaced tabs and trailing whitespace, so this should be good.  On my  
own code, I run indent as per dev spec, but afraid that if i do that  
on the rtp.[ch], it will change too much.  I'll look at getting clean- 
diff to work..

> [...]
>> @@ -298,6 +290,7 @@
>>          case CODEC_ID_MP2:
>>          case CODEC_ID_MP3:
>>          case CODEC_ID_MPEG4:
>> +        case CODEC_ID_H264:
>>              st->need_parsing = 1;
>
> ideally this should not be in this patch though i wont reject it  
> cause of
> that, its pretty harmless ..

#ifdef'd out.

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

> [...]
>
>>              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) {
>> -            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;
>> +            }
>
> is it neccessary to have these duplicated in 2 structs?

Unfortunately, because of the order of initialization events, it is.   
I don't have the context when i have to create the data and when I  
get the procs.

> [...]
>> // these statistics are used for rtcp receiver reports...
>> typedef struct {
>>     uint16_t max_seq;           ///< highest sequence number seen
>>     uint32_t cycles;            ///< shifted count of sequence  
>> number cycles
>>     uint32_t base_seq;          ///< base sequence number
>>     uint32_t bad_seq;           ///< last bad sequence number + 1
>>     uint32_t probation;         ///< sequence packets till source  
>> is valid
>>     uint32_t received;          ///< packets received
>>     uint32_t expected_prior;    ///< packets expected in last  
>> interval
>>     uint32_t received_prior;    ///< packets received in last  
>> interval
>>     uint32_t transit;           ///< relative transit time for  
>> previous packet
>>     uint32_t jitter;            ///< estimated jitter.
>> } RTPStatistics;
>
> this ideally belongs to a later statistics patch

Removed.



-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061023/1d0af040/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/20061023/1d0af040/attachment.obj>
-------------- next part --------------