[gstreamer-bugs] [Bug 509311] [rtph263pay] rtph263pay does not follow rfc2190

[GStreamer (bugzilla.gnome.org)]

If you have any questions why you received this email, please see the text at
the end of this email. Replies to this email are NOT read, please see the text
at the end of this email. You can add comments to this bug at:
  http://bugzilla.gnome.org/show_bug.cgi?id=509311

  GStreamer | gst-plugins-good | Ver: git




------- Comment #25 from Youness Alaoui  2009-04-07 21:43 UTC -------
Created an attachment (id=132297)
 --> (http://bugzilla.gnome.org/attachment.cgi?id=132297&action=view)
patch over latest patch

Hi,
First, thanks for your awesome work! Here are my comments about the latest
patch :
1 - mode A is broken because there are a few #if 0 in there to remove mode A
support.. I know you're currently testing mode B, but it doesn't have to break
mode A...
2 - Choosing the mode is important because I'm trying to make this work with
Window's Live Messenger compatibility and they only support mode A.. but when I
try with your code, it forces the use of mode B and I don't know exactly how it
decides which mode to use, I would want to force it to always use mode A.
3 - there's a memory leak in gst_rtp_h263_pay_gobfinder, you free the
GstRtpH263PayBoundry when you can't find any GOB, but you don't free it when
you actually find the GOB.
4 - the marker was always set to TRUE because you commented out the lines that
check wheter it should be set to TRUE or not (probably for debugging and forgot
to revert)
5 - If the rtp-payload-size property on ffenc_h263 isn't set to "1", then the
payloader won't work, because it loops trying to find each GOB one by one, and
if it doesn't find one, it will give the warning :
"No GOB's were found in data stream! Please enable RTP mode in encoder. Forcing
mode A for now."
and it will send the entire frame... the problem is that if you set the
rtp-payload-size to 1024 for example, you may have only 2 or 3 gob headers in
your stream and the payloader should split the packet on those gobs, and not
try to find each gob individually then add them one by one and see if the size
is < mtu. Besides, the warning says that it couldn't find any GOBs, while it
actually found quite a few, but it didn't find them *all*.
6 - I don't think that the payloader should require the encoder to add the GOB
headers since these are completely optional. What it should do is to actually
scan the stream and find the GOBs without the header (I think you already do
that for the B mode), and use that to split the message using the sbit and ebit
header fields.
I say this because WLM doesn't use GOB headers, it just splits it at GOB
boundary and uses the sbit and ebit fields.
If the code is already there for mode B, then I think it should be used also
for mode A and find the gob boundaries correctly without relying on the GOB
headers being there.

I'm attaching a patch to the previous patch (so apply the latest patch then
apply mine). I did it that way so you can see what I changed from your code.

Thanks again for your work!
KaKaRoTo


-- 
See http://bugzilla.gnome.org/page.cgi?id=email.html for more info about why you received
this email, why you can't respond via email, how to stop receiving
emails (or reduce the number you receive), and how to contact someone
if you are having problems with the system.

You can add comments to this bug at http://bugzilla.gnome.org/show_bug.cgi?id=509311.