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 758385 - [PATCH] souphttpclientsink: add "aggregate-gops" property to process GOPs as a whole
[PATCH] souphttpclientsink: add "aggregate-gops" property to process GOPs as ...
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Mac OS
: Normal enhancement
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-20 11:05 UTC by minfrin
Modified: 2018-01-28 21:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
souphttpclientsink: add "aggregate-gops" property to process GOPs as a whole (11.04 KB, patch)
2015-11-20 11:05 UTC, minfrin
rejected Details | Review

Description minfrin 2015-11-20 11:05:05 UTC
Created attachment 315951 [details] [review]
souphttpclientsink: add "aggregate-gops" property to process GOPs as a whole

This property can be used to make sure that each file part starts cleanly with
a key frame and the appropriate headers.

In order for this property to work correctly, upstream elements should make
sure than any headers that need to be written in a standalone file are:
1) in the streamheader caps field
2) and/or in the stream as one or more buffers marked with GST_BUFFER_FLAG_HEADER
   that are just before the keyframe buffer

This is useful for MPEG-TS/MPEG-PS file segmenting in
combination with mpegtsmux or mpegpsmux.

Based on original patch for multifilesink:

http://lists.freedesktop.org/archives/gstreamer-commits/2015-May/087077.html
Comment 1 Tim-Philipp Müller 2015-11-20 15:57:49 UTC
Could you explain the use case for this? Why are there  multiple "file parts" here?
Comment 2 minfrin 2015-11-23 10:58:03 UTC
I can't explain it, no.

You can find the explanation in the original commit for multifilesink described above:

http://lists.freedesktop.org/archives/gstreamer-commits/2015-May/087077.html

This patch is required as a prerequisite for support for using souphttpclient as a sink for HLS, see this bug for further information: https://bugzilla.gnome.org/show_bug.cgi?id=758453
Comment 3 Tim-Philipp Müller 2015-11-23 11:12:22 UTC
Let's just continue discussion of this in the other bug then :)

*** This bug has been marked as a duplicate of bug 758453 ***
Comment 4 minfrin 2015-11-23 11:17:42 UTC
This bug isn't a duplicate of https://bugzilla.gnome.org/show_bug.cgi?id=758453, this is a completely separate patch.

Would it be possible to clarify why you would like to combine them?
Comment 5 Tim-Philipp Müller 2015-11-23 11:30:26 UTC
My understanding was that both patches work towards the same use case you want to enable.

Both patches are basically 'move multifilesink functionality into souphttpclientsink', no?

I'm inclined to reject both patches in favour of a different approach, as mentioned in the other bug.

It's a bit curious that you can't explain the use case for a patch you propose, but perhaps I misunderstood what you meant :)
Comment 6 minfrin 2015-11-23 11:44:34 UTC
The patches are separate to make them easier to review. I can combine them, but it will be a big patch.
Comment 7 Tim-Philipp Müller 2015-11-23 12:01:36 UTC
There's no need to combine them, you can just attach it as a separate patch to the other bug.
Comment 8 minfrin 2015-12-04 14:35:55 UTC
The patch in 758453 has been withdrawn, reopening this one.
Comment 9 minfrin 2015-12-04 14:37:54 UTC
Having pulled this functionality apart, what the aggregate-gops option does is ensure that all attempts to write to the sink (in this case a PUT request, in the filesink's case a disk write) are written as complete group-of-pictures, and you don't get partial writes of parts of a gop.
Comment 10 Tim-Philipp Müller 2015-12-04 14:54:23 UTC
Comment on attachment 315951 [details] [review]
souphttpclientsink: add "aggregate-gops" property to process GOPs as a whole

Thanks for the patch, but I'm not too keen on duplicating this code everywhere, there are lots of other sink elements where someone might ask for the same thing.

I'd rather see a gopaggregate element or such that accumulates GOPs and puts them into a single buffer list or merged buffer, or alternatively perhaps something in the base sink class (not sure about option 2).
Comment 11 minfrin 2015-12-04 15:06:34 UTC
Is there a practical way of passing a buffer list to a sink generically?
Comment 12 Tim-Philipp Müller 2015-12-04 15:15:56 UTC
yes, gst_pad_push_list()