GNOME Bugzilla – Bug 681327
[0.11] getters accessors for buffer pool and memory allocator for base classes
Last modified: 2012-09-24 18:27:35 UTC
Sometimes the decoder would need to use the pool or the allocator for something else than just allocating output buffers. For example, the querying for different parameters, such as asking for a bigger number of buffers to allocate in the pool. I think it would be nice to have these two getter accessors, either for the buffer pool and the allocator.
Created attachment 220483 [details] [review] getter accessors for buffer pool and allocator in video decoder This patch expose a two getters accessors: one for the buffer pool and the other for the memory allocator.
Review of attachment 220483 [details] [review]: Good idea in general, but for consistency the same should be done for the video encoder base class, the audio decoder/encoder base classes, basetransform, basesrc, basesink too. Could you prepare the same patches for them too? :) ::: gst-libs/gst/video/gstvideodecoder.c @@ +3075,3 @@ + g_return_val_if_fail (GST_IS_VIDEO_DECODER (decoder), NULL); + + return decoder->priv->pool; This should return a new reference to the pool @@ +3089,3 @@ + g_return_val_if_fail (GST_IS_VIDEO_DECODER (decoder), NULL); + + return decoder->priv->allocator; This should return a new reference to the allocator and the allocation parameters, e.g. void gst_video_decoder_get_allocator (GstVideoDecoder * decoder, GstAllocator ** allocator, GstAllocationParams * params)
(In reply to comment #2) > Review of attachment 220483 [details] [review]: > > Good idea in general, but for consistency the same should be done for the video > encoder base class, the audio decoder/encoder base classes, basetransform, > basesrc, basesink too. Could you prepare the same patches for them too? :) Sure! Let me grab some coffee and I'll cook those patches...
Created attachment 220581 [details] [review] getter accessor for buffer pool and allocator in basesr
Review of attachment 220581 [details] [review]: ::: libs/gst/base/gstbasesrc.c @@ +3632,3 @@ + g_return_val_if_fail (GST_IS_BASE_SRC (src), NULL); + + return src->priv->pool; You need to return a new reference to the pool here @@ +3646,3 @@ + g_return_val_if_fail (GST_IS_BASE_SRC (src), NULL); + + return src->priv->allocator; You need to return a new reference to the allocator here. And also the GstAllocationParams, i.e. gst_base_src_get_allocator (GstBaseSrc *src, GstAllocator **allocator, GstAllocationParams *params)
Created attachment 220582 [details] [review] getter accessor for buffer pool and allocator in base transform
Created attachment 220583 [details] [review] getter accessor for allocator in audio decoder
(In reply to comment #5) > Review of attachment 220581 [details] [review]: > > ::: libs/gst/base/gstbasesrc.c > @@ +3632,3 @@ > + g_return_val_if_fail (GST_IS_BASE_SRC (src), NULL); > + > + return src->priv->pool; > > You need to return a new reference to the pool here > > @@ +3646,3 @@ > + g_return_val_if_fail (GST_IS_BASE_SRC (src), NULL); > + > + return src->priv->allocator; > > You need to return a new reference to the allocator here. And also the > GstAllocationParams, i.e. > gst_base_src_get_allocator (GstBaseSrc *src, GstAllocator **allocator, > GstAllocationParams *params) mmmhh... That would be needed in all of my patches then...
Sorry for that :( That's why I mentioned it in comment #2 already
(In reply to comment #9) > Sorry for that :( That's why I mentioned it in comment #2 already Sorry!! I didn't see it!!! My mistake... This afternoon I'll work on this.. Thanks!
Please, be patient with these patches: my testing work bench went bananas and I'm rebuilding it from scratch :)
Created attachment 220875 [details] [review] getter accessors for buffer pool and allocator in video decoder
Created attachment 220876 [details] [review] getter accessor for allocator in video encoder
Created attachment 220878 [details] [review] getter accessor for allocator in audio encoder
Created attachment 220880 [details] [review] getter accessor for allocator in audio decoder
Created attachment 220881 [details] [review] getter accessor for buffer pool and allocator in basesrc
Created attachment 220882 [details] [review] getter accessor for buffer pool and allocator in base transform
Created attachment 221002 [details] [review] getter accessor for buffer pool and allocator in base transform if param is NULL there's no need to initialize it
Created attachment 221003 [details] [review] getter accessor for buffer pool and allocator in basesrc if param is NULL, don't initialize it
Created attachment 221004 [details] [review] getter accessor for allocator in audio decoder if param is null, don't initialize it
Created attachment 221006 [details] [review] getter accessor for allocator in audio encoder if param is null don't initialize it
Created attachment 221007 [details] [review] getter accessor for allocator in video encoder if param is null, don't initialize it
Created attachment 221008 [details] [review] getter accessors for buffer pool and allocator in video decoder if param is null don't init it
Created attachment 221110 [details] [review] getter accessors for buffer pool and allocator in video decoder
Created attachment 221111 [details] [review] getter accessor for allocator in video encoder
Created attachment 221113 [details] [review] getter accessor for allocator in audio encoder
Created attachment 221114 [details] [review] getter accessor for allocator in audio decoder
Created attachment 221115 [details] [review] getter accessor for buffer pool and allocator in basesrc
Created attachment 221116 [details] [review] getter accessor for buffer pool and allocator in base transform
Looks good, except that it should be _get_buffer_pool() instead of _get_pool(). Do you want to change it? Otherwise I'll do it before pushing the patches.
I can do it right now :)
Created attachment 221134 [details] [review] getter accessor for buffer pool and allocator in basesrc renamed _get_pool() to _get_buffer_pool()
Created attachment 221135 [details] [review] getter accessor for buffer pool and allocator in base transform renamed _get_pool() to _get_buffer_pool()
Created attachment 221136 [details] [review] getter accessors for buffer pool and allocator in video decoder renamed _get_pool() to _get_buffer_pool()
(In reply to comment #30) > Looks good, except that it should be _get_buffer_pool() instead of _get_pool(). > Do you want to change it? Otherwise I'll do it before pushing the patches. Updated!