[Bug 705079] hlsdemux: switch fragments to buffer lists to avoid copies

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=705079
  GStreamer | gst-plugins-bad | 1.1.2

--- Comment #2 from Arnaud Vrac <rawoul at gmail.com> 2013-07-30 10:15:54 UTC ---
(In reply to comment #1)
> Review of attachment 250372 [details]:
> 
> ::: ext/hls/gsthlsdemux.c
> @@ +1322,3 @@
> +
> +    /* Make sure we are aligned to the cipher block size */
> +    buf_size &= ~(buf_size & (block_size - 1));
> 
> Doesn't this cause us to decrypt parts of the buffer twice?

No, this clips the buffer size to be a multiple of the block size, the left
bytes are copied in the temporary block, and when it is filled and decrypted
the data is copied back to the buffers. This is a little convoluted but I don't
see how to do it without copying or allocating too much data.

> 
> @@ +1342,3 @@
> +      /* Handle pkcs7 unpadding here */
> +      gsize unpadded_size = map.size - map.data[map.size - 1];
> +      gst_buffer_resize (buffer, 0, unpadded_size);
> 
> In theory the unpadding could remove one or more buffers from the list in the
> end.

Right, I'll change this.

> 
> ::: gst-libs/gst/uridownloader/gstfragment.c
> @@ +47,2 @@
>    GstCaps *caps;
> +  gsize size;
> 
> Why not make the size a property too?
> 

Ok.

> @@ +245,3 @@
> +    buffer = gst_buffer_list_get (fragment->priv->buffer_list, 0);
> +    if (buffer) {
> +      caps = gst_type_find_helper_for_buffer (NULL, buffer, NULL);
> 
> This could mean that you only typefind on a very small buffer, which might fail just because of that

Yes, I've looked for a function in typefind that tells me the minimal amount of
data needed to do proper typefinding but I couldn't find one, like the
max-extent in shared-mime-info.

In the end the caps shouldn't be detected like this, the allowed data types are
defined in the HLS spec so the caps should be set manually depending of the
playlist info.

> 
> ::: gst-libs/gst/uridownloader/gstfragment.h
> @@ +61,3 @@
>  GType gst_fragment_get_type (void);
> 
> +GstBufferList * gst_fragment_get_buffer_list (GstFragment *fragment);
> 
> This also needs changes in dashdemux and mssdemux

Ok, I should have grepped for other users.

-- 
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.