[FFmpeg-devel] MPEG Video support in RTP

[Michael Niedermayer]

Hi

On Fri, Aug 24, 2007 at 10:11:50AM +0200, Luca Abeni wrote:
> Hi,
>
> Luca Abeni wrote:
>> Hi Luca,
>> Luca Barbato wrote:
>> [...]
>>>> ops... Sorry, I've been too fast in sending the patch. I updated my 
>>>> local copy with the headers from rtp.{c,h}
>>>>
>>> If is still working as supposed please commit =)
>> Ok, thanks. I'll wait one day or two to give other people the possibility 
>> to comment, and then I'll commit.
>
> Ok, committed. Now, here are the interesting patches:
>
> - remove_header_extension.diff: the current code writes an empty payload 
> header extension if the video is MPEG2. However, this header is wrong 
> (filled with all 0s), and according to the standard "its inclusion in an 
> RTP packet is optional". Since removing this header we are still compliant 
> with RFC2250 (even in case of MPEG2 video), this patch is removing it.

id say looks ok unless our rtsp maintainer disagrees


>
> - fix_packetization.diff: this is the biggest patch. It ensures that a 
> video frame is correctly split on frame boundaries, according to RFC2250. 
> Note: the patch assumes that video sequence header, gop header, and picture 
> header if present are at the beginning of a frame (I think this is 
> guaranteed by the MPEG standard, right?), 

yes


> and that the first packet is big 
> enough to contain such headers.
> After applying this patch, the payload header is still wrong, but the frame 
> should be correctly packetized, and we have all the information needed to 
> fill the header. The patch is more complex than I would have liked, but 
> I've not been able to come out with something simpler than this... 
> Suggestions are welcome.
>

[...]

> - set_frame_type.diff: set the P field in the payload header. Note: in the 
> patch, I assume that the frame is well-formed (I consider writing a 
> non-well-formed frame similar to setting a wrong "size" field in AVPacket). 
> Is this reasonable? If not, I'll add a check when reading frame_type

yes you can assume that the stream is valid mpeg2 ...

[...]
>      while (size > 0) {
>          /* XXX: more correct headers */
>          h = 0;
>          q = s->buf;
> +        len = FFMIN(max_packet_size - (q - s->buf), size);
> +        if (len == size) {
> +            end_of_slice = 1;
> +        } else {
> +            const uint8_t *r, *r1;
> +            int start_code, done;
> +
> +            r1 = buf1;
> +            done = 0;
> +            while (!done) {
> +                start_code = -1;
> +                r = ff_find_start_code(r1, buf1 + size, &start_code);
> +                if((start_code & 0xFFFFFF00) == 0x100) {
> +                    /* New start code found */
> +                    if (r - buf1 - 4 < len) {
> +                        /* The current slice fits in the packet */
> +                        if (begin_of_slice == 0) {
> +                            /* no slice at the beginning of the packet... */
> +                            done = 1;
> +                            end_of_slice = 1;
> +                            len = r1 - buf1 - 4;
> +                        }
> +                        r1 = r;
> +                    } else {
> +                        done = 1;
> +                    }
> +                } else {
> +                    done = 1;
> +                }
> +            }
> +        }

if i understand this correctly it will never put more than 1 slice in a
packet, this is bad in case slices are small VERY bad
mpeg2 _requires_ 1 slice per MB row at minimum so a 352x288 mpeg 2 video
at 25fps would need 450 packets per second even if its just lets say
100kbit/sec thats 28 byte of the mpeg2 per packet on average ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070827/96d0e0ed/attachment.pgp>