[FFmpeg-devel] RTP mark bit not passed to parse_packet

[Alexandre FERRIEUX - FT/RD/SIRP/ASF/SOFTL]

Hi Luca,

First thanks for the fast response !

>> In the process of writing an RTP depayloader for H263 (RFC2429, simple 
>> follow-on packet mode), I see that the current API doesn't pass the RTP 
>> Mark bit (buf[1]&0x80) to the payload handler callback.
> 
> Yes, the parse_packet() calback looks quite strange to me too...
> It is not considering important information (like the M bit) and
> has some other "interesting features", like the "*timestamp"
> parameter...

OK. Glad to see we share the feeling :)

> 
> I think the RTP de-packetiser is not supposed to split the stream in
> frames, but simply to simply return the payload. libavcodec then contains
> a parser (see libavcodec/h263_parser.c) that is in charge of splitting the
> stream in frames. You just have to ensure that rtp_parse_open() sets
> st->need_parsing = AVSTREAM_PARSE_FULL for H.263 too (as it already does
> for mpeg video, and audio, H.264, etc...).

Thanks, that is indeed the key info I was missing.

Discovering this, I realize I may not have read all the relevant 
documentation. I apologize for this. I'd appreciate a pointer to the 
part describing the interaction between the encapsulation and codec layers.

In the meantime, just one point I'd like to clarify regarding H263:
The follow-on-packet method just uses a payload-header of 2 bytes, with 
values 0x0400 for the first packet (P bit set) and 0x0000 for 
follow-ons. In addition, the P bit in the first packet "encode" two zero 
bytes of the subsequent bitstream (which are thus removed on 
encapsulation). Then, when depacketizing the first packet, should I pass 
to the decoder:

	- the 0400 header + bitstream without 0000
	- the 0400 header + bitstream with 0000
         - bitstream with 0000 but no payload header

(and similar question for follow-ons).

> I agree that this should be improved (the current design is not robust
> against packet loss, for example), but this is how things currently work
> (as far as I understand). Anyway, I think that patches in this regard
> are highly welcome ;-)

I'll be glad to contribute once I get it right :)

But here is already what I did on the other side, to allow ffmpeg to 
stream H263 out. Of course it is just the simple follow-on method, hence 
no error resilience. It is notably less ambitious than what is currently 
done for H264; but I have the plan to improve it later).

I do realize this is naive code, and I'd be happy to see you take over. 
It's just that I found it a bit frustrating to see the H263+ codec 
supported but virtually nothing on the RTP side...


HTH,

-Alex



Index: rtpenc.c
===================================================================
--- rtpenc.c    (revision 16614)
+++ rtpenc.c    (working copy)
@@ -243,6 +243,47 @@
      }
  }

+static void rtp_send_h263p(AVFormatContext *s1,
+                                                  const uint8_t *buf1, 
int size)
+{
+  RTPDemuxContext *s = s1->priv_data;
+  int maxs,first,len;
+  char locbuf[65536]; /* overestimation of MTU !!! */
+
+  /* RFC2429 payload header == 16 bits */
+
+  maxs = s->max_payload_size - 2 - 32 /* margin */;
+
+  /* we remove the two leading zeroes as per P bit */
+
+  buf1+=2;
+  size-=2;
+
+  first=1;
+  while(size>0)
+       {
+         len = size;
+         if (len > maxs)
+               len = maxs;
+
+         /* simple payload header, no-resilience mode: 0400 for 
beginning, 0000 for following */
+         if (first)
+               {
+                 first=0;
+                 locbuf[0]=4;
+               }
+         else
+               {
+                 locbuf[0]=0;
+               }
+         locbuf[1]=0;
+         memcpy(locbuf + 2, buf1, len);
+         ff_rtp_send_data(s1, locbuf, len + 2, (len == size));
+         size -= len;
+         buf1 += len;
+       }
+}
+
  static void rtp_send_raw(AVFormatContext *s1,
                           const uint8_t *buf1, int size)
  {
@@ -339,6 +380,10 @@
      case CODEC_ID_H264:
          ff_rtp_send_h264(s1, buf1, size);
          break;
+    case CODEC_ID_H263:
+    case CODEC_ID_H263P:
+        rtp_send_h263p(s1, buf1, size);
+        break;
      default:
          /* better than nothing : send the codec raw data */
          rtp_send_raw(s1, buf1, size);
Index: rtp.c
===================================================================
--- rtp.c       (revision 16614)
+++ rtp.c       (working copy)
@@ -70,7 +70,8 @@
    {32, "MPV",        CODEC_TYPE_VIDEO,   CODEC_ID_MPEG1VIDEO, 90000, -1},
    {32, "MPV",        CODEC_TYPE_VIDEO,   CODEC_ID_MPEG2VIDEO, 90000, -1},
    {33, "MP2T",       CODEC_TYPE_DATA,    CODEC_ID_MPEG2TS, 90000, -1},
-  {34, "H263",       CODEC_TYPE_VIDEO,   CODEC_ID_H263, 90000, -1},
+  /*  {34, "H263",       CODEC_TYPE_VIDEO,   CODEC_ID_H263, 90000, -1}, */
+  {96, "H263",       CODEC_TYPE_VIDEO,   CODEC_ID_H263, 90000, -1},
    {-1, "",           CODEC_TYPE_UNKNOWN, CODEC_ID_NONE, -1, -1}
  };