After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 659915 - MP2T RTP payloader rejects buffers exceeding MTU
MP2T RTP payloader rejects buffers exceeding MTU
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.32
Other All
: Normal normal
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-09-23 06:34 UTC by mpaklin
Modified: 2012-05-18 10:58 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description mpaklin 2011-09-23 06:34:14 UTC
Is RTP payloader element supposed to handle buffers that exceed their configured MTU?
I thought that it should - it uses GstAdapter for that. However for some reason buffers that are larger than the MTU are outright rejected on input.

A simple test to confirm it shows

gst-launch filesrc blocksize=20000 location=test.ts ! rtpmp2tpay mtu=10000 !
fakesink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...

** (gst-launch-0.10:5332): CRITICAL **: file
..\..\..\Source\gstreamer\libs\gst\base\gstadapter.c: line 450: assertion ` size > 0' failed Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
Got EOS from element "pipeline0".
Execution ended after 5456312000 ns.
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...


While changing block size from 20000 to 2000 works just fine.

gst-launch filesrc blocksize=2000 location=test.ts ! rtpmp2tpay mtu=10000 !
fakesink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
Got EOS from element "pipeline0".
Execution ended after 3134179000 ns.
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...


The cause for this is in gst_basertppayload_is_filled(), which does the following
  if (size > payload->mtu)
    return TRUE;
Claiming that the payload is "full" because the input size exceeds MTU. This does not look right to me, but the check is obviously intentional.
The fact that the payload is full triggers the flushing, which results in an attempt to send an empty payload and that is the root cause the assert shown above.

The question that I have - is this behavior by design?
Do I expect it to do something that it is not designed for?

The test was done with the same results on Fedora Core 15 (0.10.32) and Windows (0.10.28).

Regards,
-- Max Paklin.
Comment 1 David Schleef 2011-09-24 21:44:58 UTC
rtpmp2t should be ignoring any ganging together of TS packets in the input, and splitting/gathering buffers as necessary to send out an MTU of packets at a time.  If it isn't, then it's a bug.

In any case, the 'CRITICAL' message indicates a bug.

It is unlikely that you really mean 0.10.32 for rtpmp2tpay, as the most recent release is 0.10.30.
Comment 2 David Schleef 2011-09-24 21:46:06 UTC
Also, this needs a confirmation that this bug exists in current release and/or git master.  It is likely I would have noticed it already.
Comment 3 mpaklin 2011-09-26 18:30:11 UTC
(In reply to comment #1)
> rtpmp2t should be ignoring any ganging together of TS packets in the input, and
> splitting/gathering buffers as necessary to send out an MTU of packets at a
> time.  If it isn't, then it's a bug.
> 
> In any case, the 'CRITICAL' message indicates a bug.
> 
> It is unlikely that you really mean 0.10.32 for rtpmp2tpay, as the most recent
> release is 0.10.30.

Sorry, David, I made a mistake with the version. What I provided you with was the version of gstreamer, not the plugins-good, which is 0.10.29.

I agree with you, this looks like a problem, which is why I filed a bug. It certainly should take packets larger than the MTU, which is why it uses GstAdapter. It should, but it does not.

As I mentioned, run this command to see for yourself.

gst-launch filesrc blocksize=20000 location=test.ts ! rtpmp2tpay mtu=10000 !
fakesink
Comment 4 mpaklin 2011-09-26 20:06:39 UTC
(In reply to comment #2)
> Also, this needs a confirmation that this bug exists in current release and/or
> git master.  It is likely I would have noticed it already.

I just pulled the latest from git and it repros there exactly as it did before.
It was pulled from 0.10 version (gstreamer 0.10.35.1, plugins 0.10.30.1).
The source looks exactly as it did before as far as the MTU check goes.

gst-launch filesrc blocksize=20000 location=test.ts ! rtpmp2tpay mtu=10000 !
fakesink
fails with the same assert, while

gst-launch filesrc blocksize=1000 location=test.ts ! rtpmp2tpay mtu=10000 !
fakesink
works fine.
Comment 5 Mark Nauwelaerts 2012-05-18 10:58:12 UTC
AFAICS, there should not have been any failure with recent versions.

However, it would not have payloaded correctly, in that it would exceed set mtu size and not arrange for rtp packets to carry an integral number of TS packets.

Following should fix this:

[0.10]
commit ee6c6e9060fd542c0f5f03bad46eb355a6754f66
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Fri May 18 12:51:29 2012 +0200

    rtpmp2tpay: respect mtu and ts packet boundaries
    
    See #659915.

[0.11]
commit 182596b3aba6fe37751074a0b492c4f58a51beeb
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Fri May 18 12:53:44 2012 +0200

    rtpmp2tpay: respect mtu and packet boundaries
    
    See #659915.