rtpL16depay failing to do depay in place

[Petr Kulhavy]

Hi,

it seems that rtpL16depay (and I guess also other rtp-depay functions) is failing to depayload in place. It allways allocates a new buffer and copies the payload.
Before opening a bugtracking entry I'm trying to understand why. I don't see anything that should prevent to use the buffer in place.

This is the log I'm seeing:

0:00:13.489419481  1060  0x1ec5150 DEBUG            rtpL16depay gstrtpL16depay.c:243:gst_rtp_L16_depay_process:<rtpl16depay0> got payload of 192 bytes
0:00:13.489596481  1060  0x1ec5150 LOG               GST_BUFFER gstbuffer.c:798:gst_buffer_new: new 0x75c05370
0:00:13.489760981  1060  0x1ec5150 LOG               GST_BUFFER gstbuffer.c:2038:gst_buffer_copy_region: new region copy 0x75c05370 of 0x75c054b0 12-192
0:00:13.490969772  1060  0x1ec5150 LOG               GST_BUFFER gstbuffer.c:514:gst_buffer_copy_into: copy 0x75c054b0 to 0x75c05370, offset 12-192/204
--
0:00:13.493352979  1060  0x1ec5150 TRACE            GST_LOCKING gstminiobject.c:177:gst_mini_object_lock: lock 0x1ecb690: state 00020000, access_mode 3
0:00:13.493546312  1060  0x1ec5150 DEBUG            GST_LOCKING gstminiobject.c:212:gst_mini_object_lock: lock failed 0x1ecb690: state 00020000, access_mode 3
0:00:13.493717020  1060  0x1ec5150 DEBUG             GST_MEMORY gstmemory.c:317:gst_memory_map: mem 0x1ecb690: lock 3 failed
0:00:13.493910270  1060  0x1ec5150 TRACE        GST_REFCOUNTING gstobject.c:249:gst_object_ref:<allocatorsysmem0> 0x1ddc010 ref 6->7
0:00:13.494091270  1060  0x1ec5150 DEBUG             GST_MEMORY gstmemory.c:138:gst_memory_init: new memory 0x1ecb578, maxsize:195 offset:0 size:192
0:00:13.494364978  1060  0x1ec5150 DEBUG        GST_PERFORMANCE gstallocator.c:462:_sysmem_copy: memcpy 192 memory 0x1ecb690 -> 0x1ecb578


And this is the command:
GST_DEBUG=9 gst-launch-1.0 audiotestsrc is-live=true wave=silence samplesperbuffer=48 ! "audio/x-raw,format=(string)S16BE,rate=(int)48000,channels=(int)2,channel-mask=(int)0x3" ! rtpL16pay ! rtpL16depay ! fakesink
2>&1 | grep -B 5 copy

It happens in gst_audio_buffer_reorder_channels() when mapping the buffer read-write.
Why is the buffer read-only? It doesn't really matter what the source is, the same happens with udpsrc.

The other thing here is that in my particular case the channels don't have to be reordered at all so the memcpy is completely redundant.

Any light shed onto this is appreciated.

Thanks
Petr

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Sebastian Dröge-3]

On Mon, 2016-10-31 at 15:32 +0100, Petr Kulhavy wrote:

> Hi,
>
> it seems that rtpL16depay (and I guess also other rtp-depay
> functions) is failing to depayload in place. It allways allocates a
> new buffer and copies the payload.
> Before opening a bugtracking entry I'm trying to understand why. I
> don't see anything that should prevent to use the buffer in place.
>
> [...]
>
> And this is the command:
> GST_DEBUG=9 gst-launch-1.0 audiotestsrc is-live=true wave=silence
> samplesperbuffer=48 ! "audio/x-
> raw,format=(string)S16BE,rate=(int)48000,channels=(int)2,channel-
> mask=(int)0x3" ! rtpL16pay ! rtpL16depay ! fakesink 
> 2>&1 | grep -B 5 copy
>
> It happens in gst_audio_buffer_reorder_channels() when mapping the
> buffer read-write.
> Why is the buffer read-only? It doesn't really matter what the source
> is, the same happens with udpsrc.

It's read-only because something else still has a reference to it. At
least the RTP buffer has (of which the payload buffer is a
"subbuffer"). Changing this would require to get rid of the RTP buffer
reference before trying to map the payload buffer writeable.

This should get a bug report, but see below

> The other thing here is that in my particular case the channels don't
> have to be reordered at all so the memcpy is completely redundant.

That part should be relatively easy to fix. Can you file a bug (one for
both issues here), ideally with a patch for this part at least?

https://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer&component=gst-plugins-good

--
Sebastian Dröge, Centricular Ltd · http://www.centricular.com
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Petr Kulhavy]

Hi Sebastian,

thank you for your reply. Bug created:
https://bugzilla.gnome.org/show_bug.cgi?id=773833

I'm not sure what is the prefered way to fix the second part.
Ideally the remap read-write would be moved into
gst_audio_buffer_reorder_channels(), but that might break some existing
API/ABI ?
Otherwise this change has to be done in every affected depay...

Cheers
Petr


On 02/11/16 08:54, Sebastian Dröge wrote:

> On Mon, 2016-10-31 at 15:32 +0100, Petr Kulhavy wrote:
>> Hi,
>>
>> it seems that rtpL16depay (and I guess also other rtp-depay
>> functions) is failing to depayload in place. It allways allocates a
>> new buffer and copies the payload.
>> Before opening a bugtracking entry I'm trying to understand why. I
>> don't see anything that should prevent to use the buffer in place.
>>
>> [...]
>>
>> And this is the command:
>> GST_DEBUG=9 gst-launch-1.0 audiotestsrc is-live=true wave=silence
>> samplesperbuffer=48 ! "audio/x-
>> raw,format=(string)S16BE,rate=(int)48000,channels=(int)2,channel-
>> mask=(int)0x3" ! rtpL16pay ! rtpL16depay ! fakesink
>> 2>&1 | grep -B 5 copy
>>
>> It happens in gst_audio_buffer_reorder_channels() when mapping the
>> buffer read-write.
>> Why is the buffer read-only? It doesn't really matter what the source
>> is, the same happens with udpsrc.
> It's read-only because something else still has a reference to it. At
> least the RTP buffer has (of which the payload buffer is a
> "subbuffer"). Changing this would require to get rid of the RTP buffer
> reference before trying to map the payload buffer writeable.
>
> This should get a bug report, but see below
>
>> The other thing here is that in my particular case the channels don't
>> have to be reordered at all so the memcpy is completely redundant.
> That part should be relatively easy to fix. Can you file a bug (one for
> both issues here), ideally with a patch for this part at least?
>
> https://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer&component=gst-plugins-good
>

_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel