[gst-devel] [gst-cvs] wtaygst-plugins-good: gst-plugins-good/gst-plugins-good/gst/rtp/
peter.kjellerstedt at axis.com
Wed Dec 12 16:46:09 CET 2007
> -----Original Message-----
> From: gstreamer-devel-bounces at lists.sourceforge.net
> [mailto:gstreamer-devel-bounces at lists.sourceforge.net] On
> Behalf Of Wai-Ming Ho
> Sent: den 12 december 2007 14:25
> To: gstreamer-devel at lists.sourceforge.net
> Subject: Re: [gst-devel] [gst-cvs] wtaygst-plugins-good:
> On Wed, 2007-12-12 at 10:28 +0100, Peter Kjellerstedt wrote:
> > > Patch by: Wai-Ming Ho <webregbox at yahoo dot co dot uk>
> > > * gst/rtp/gstrtph264pay.c: (gst_rtp_h264_pay_init),
> > > (gst_rtp_h264_pay_finalize), (gst_rtp_h264_pay_setcaps),
> > > (next_start_code), (is_nal_equal), (gst_rtp_h264_pay_decode_nal),
> > > (encode_base64), (gst_rtp_h264_pay_parse_sps_pps),
> > > (gst_rtp_h264_pay_handle_buffer):
> > > * gst/rtp/gstrtph264pay.h:
> > > Use higher performance start-code searching.
> > > Parse NALs and store SPS, PPS and profile in the caps so
> > > that they can be used in the SDP. Fixes #502814.
> > >
> > > Modified files:
> > > . : ChangeLog
> > > gst/rtp : gstrtph264pay.c gstrtph264pay.h
> > >
> > > Links:
> > >
> > >
> > >
> > * In gst_rtp_h264_pay_finalize():
> > Why check for NULL before calling g_free()?
> > * In encode_base64():
> > Why implement yet another base64-encoder? Now there is one
> > in the RTSP-code and one here. Wouldn't it make sense to have
> > one implementation for all of GStreamer? And if glib >= 2.12
> > then I guess you could use g_base64_encode(), and not need
> > to have any at all.
> Pls check with Wim Taymans. The original patch proposed on
> Bugzilla uses g_base64_encode().
Unfortunately I know the reason for not using g_base64_encode().
GStreamer seems to be stuck at only requiring glib 2.8, and
g_base64_encode() was not added to glib until 2.12.
> > * Why whould is_nal_equal() be faster than memcmp()?
> > On what architectures?
> > Oh, and the comment preceding it mentions memcpy rather
> > than memcmp()...
> fingers typing error :-) it's suppose to read memcmp()
> I tested it on Ubuntu 7.04 /Intel Genuine Intel(R) CPU 2140 @ 1.60GHz
> stepping 02. when I proposed the patch, is_nal_equal() is either the
> patched implementation or a simple call to memcmp().
> As for why, I would have to look at the gcc generated code but a
> quick guess would be the 32-bit XOR's instead of byte-wise CMP
> (since memcmp() needs to return either equal, less or greater).
I very much doubt memcmp() is implemented as a simple byte-wise
compare. I know that uClibc (which we use) use a much more
elaborate memcmp() comparing two unsigned long at a time, and
since the memcmp code seems to be taken from glibc, glibc would
> Other than that, if the code is not critical performance hit,
> I would be pretty happy if it was a simple call to memcmp().
> The gain of 20% is observed only when both blocks are identical
> most (or all in the case os single SPS/PPS H264 bitstreams) of
> the time.
I am not sure the potential performance increase is worth all
the extra complexity compared to a simple memcmp(). After all
the GStreamer code does a lot of memory allocations and memory
copying all over the place. It is not really optimised at this
> > * Why not make gst_rtp_h264_pay_decode_nal() return a gboolean
> > denoting the updated status, rather than making it take a
> > gboolean return parameter?
> That would be a good idea.
> > //Peter
More information about the gstreamer-devel