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 654585 - rtpmp4gdepay choppy sound
rtpmp4gdepay choppy sound
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other Linux
: Normal normal
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-07-14 00:49 UTC by gudake
Modified: 2011-09-06 12:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fixes rtpmp4gdepay (629 bytes, patch)
2011-07-14 00:49 UTC, gudake
committed Details | Review
patch (1.00 KB, patch)
2011-07-15 01:36 UTC, gudake
reviewed Details | Review

Description gudake 2011-07-14 00:49:03 UTC
play 
rtsp://ushqmed01.palm.com/rtsp_non-QT/video/3GP/3gp_H264_AAC-LC/H264_176x144_25fps_0100kbps_AAC-LC_048kbps_44khz_M.3gp

on latest gstreamer release, the sound is choppy.

See attached fix
Comment 1 gudake 2011-07-14 00:49:54 UTC
Created attachment 191935 [details] [review]
fixes rtpmp4gdepay
Comment 2 David Schleef 2011-07-14 20:15:52 UTC
Could you resubmit the patch in 'git format-patch' format?  It makes it easier to review and automatically carries the submitter and changelog information.

"latest gstreamer release" -- is this a regression?
Comment 3 gudake 2011-07-15 01:35:48 UTC
This bug didn't occur before 2008/9/15 when the interleaved AU supported was added.
Comment 4 gudake 2011-07-15 01:36:27 UTC
Created attachment 192014 [details] [review]
patch
Comment 5 Tim-Philipp Müller 2011-07-16 23:27:36 UTC
Comment on attachment 192014 [details] [review]
patch

>@@ -611,6 +611,8 @@ gst_rtp_mp4g_depay_process (GstBaseRTPDepayload * depayload, GstBuffer * buf)
>               rtpmp4gdepay->next_AU_index = GST_BUFFER_OFFSET (outbuf);
>               gst_rtp_mp4g_depay_flush_queue (rtpmp4gdepay);
>             }
>+            // rebase next_AU_index to current rtp's first AU_index
>+            rtpmp4gdepay->next_AU_index = AU_index;
>           }
>           rtpmp4gdepay->prev_rtptime = rtptime;
>           rtpmp4gdepay->prev_AU_num = num_AU_headers;

Without knowing the details of the code, it looks to me like one of these two rtpmp4gdepay->next_AU_index assignments is either wrong or superfluous (why assign it to A if it's going to be assigned to B two lines below?). Also, please don't use C++-style comments, use /* blah */ -style comments
Comment 6 Mark Nauwelaerts 2011-09-06 12:25:50 UTC
Thanks.  Turned it into a commit this way.

W.r.t. to the assignments as marked above, there is some stuff happening in the function call (gst_rtp_mp4g_depay_flush_queue) that 'interacts' with the assignments.

commit 06f8e356a6829e067e81e8c2bf7bac334304a44b
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Tue Sep 6 13:18:40 2011 +0200

    rtpmp4gdepay: improve bogus interleaved index compensating
    
    Patch by <gudake@gmail.com>
    
    Fixes #654585.
Comment 7 Mark Nauwelaerts 2011-09-06 12:26:45 UTC
Hm, obviously missed the second attachment here :(