[Bug 740945] directshowdec: Port decoder plugin to 1.x

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=740945
  GStreamer | gst-plugins-bad | 1.x

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #292142|none                        |needs-work
             status|                            |

--- Comment #7 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-12-04 18:53:35 UTC ---
Review of attachment 292142:
 --> (https://bugzilla.gnome.org/review?bug=740945&attachment=292142)

Perfect, thanks for updating :)

::: sys/dshowdecwrapper/gstdshowaudiodec.cpp
@@ +211,3 @@
     GST_BUFFER_DURATION (out_buf) = duration;
+
+    if (gst_buffer_fill(out_buf, 0, pBuffer, size) != size) {

Now only this seems a bit problematic. As you see, getting a buffer from the
pool does not allow you to set the size. I assume this will always be fine
currently because you set the size of the pool to GetBufferSize() when you
create a pool yourself (and downstream will usually never provide one for
audio).

But if you ever get a buffer of a different size here, or if downstream
provides a buffer pool to you, this is probably going to break in one way or
another.


I would recommend for now to disable all the buffer pool magic as for audio we
don't use them yet... exactly for the reason mentioned above. No downstream
will currently provide you a buffer pool with audio either for that reason.

So better comment out the allocation query stuff below, and just do a
gst_buffer_new_allocate() above with the correct size.

@@ +979,3 @@
+    gst_query_parse_nth_allocation_pool (query, 0, &pool, &pool_size,
&pool_min, 
+      &pool_max);
+  }

You should also handle the allocators from downstream here and set them on the
pool... if any.

If you get an allocator and don't want to use a pool, you could btw use
gst_buffer_new_allocate() with that allocator. That is something that will more
likely happen for audio, and only that case is currently handled in the
GstAudioDecoder base class (that is: downstream pools are ignored, but if there
is an allocator we're going to use it).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.