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 754435 - flvmux: use time segment and copy timestamps when streamable
flvmux: use time segment and copy timestamps when streamable
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-09-02 08:50 UTC by Håvard Graff (hgr)
Modified: 2015-10-11 10:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch with test (9.09 KB, patch)
2015-09-02 08:50 UTC, Håvard Graff (hgr)
committed Details | Review
additional patch (2.93 KB, patch)
2015-09-23 16:38 UTC, Håvard Graff (hgr)
committed Details | Review

Description Håvard Graff (hgr) 2015-09-02 08:50:54 UTC
Created attachment 310473 [details] [review]
patch with test

Ref. discussion with slomo, __tim and stormer.
Comment 1 Sebastian Dröge (slomo) 2015-09-02 09:28:04 UTC
Review of attachment 310473 [details] [review]:

::: gst/flv/gstflvmux.c
@@ +1126,3 @@
+    } else {
+      GST_BUFFER_OFFSET (tag) = GST_BUFFER_OFFSET (buffer);
+      GST_BUFFER_OFFSET_END (tag) = GST_BUFFER_OFFSET_END (buffer);

Copying the offsets seem not very useful either here. They are going to be values specific to the input streams, and not the offset in the output.
Comment 2 Håvard Graff (hgr) 2015-09-02 09:32:53 UTC
(In reply to Sebastian Dröge (slomo) from comment #1)
> Review of attachment 310473 [details] [review] [review]:
> 
> ::: gst/flv/gstflvmux.c
> @@ +1126,3 @@
> +    } else {
> +      GST_BUFFER_OFFSET (tag) = GST_BUFFER_OFFSET (buffer);
> +      GST_BUFFER_OFFSET_END (tag) = GST_BUFFER_OFFSET_END (buffer);
> 
> Copying the offsets seem not very useful either here. They are going to be
> values specific to the input streams, and not the offset in the output.

I did not add this, this was already there. I just added copying of timestamps when streamable.
Comment 3 Nicolas Dufresne (ndufresne) 2015-09-02 12:41:38 UTC
Hmm, true, I should have remove that too. Offset is in FORMAT_DEFAULT, do other muxers convert that to bytes offset ?
Comment 4 Håvard Graff (hgr) 2015-09-23 16:38:47 UTC
Created attachment 311960 [details] [review]
additional patch

In order for flvdemux to correctly parse the stream, the offsets need to be NONE.
Comment 5 Sebastian Dröge (slomo) 2015-10-11 10:11:05 UTC
Comment on attachment 311960 [details] [review]
additional patch

commit b6f133ba1794cdb39cc9c9892785a907b617323a
Author: Havard Graff <havard.graff@gmail.com>
Date:   Tue Sep 22 22:59:16 2015 +0200

    flvmux: GST_BUFFER_OFFSETs should be GST_BUFFER_OFFSET_NONE
    
    Or else flvdemux don't understand it
    
    https://bugzilla.gnome.org/show_bug.cgi?id=754435
Comment 6 Sebastian Dröge (slomo) 2015-10-11 10:11:10 UTC
Comment on attachment 310473 [details] [review]
patch with test

commit cf3a2294da194727027d498b85e12ef49fadb440
Author: Havard Graff <havard.graff@gmail.com>
Date:   Wed Sep 2 10:44:59 2015 +0200

    flvmux: use time segment and copy timestamps when streamable
    
    Add a basic test using speex data to verify timestamping.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=754435
Comment 7 Sebastian Dröge (slomo) 2015-10-11 10:39:33 UTC
Same change for matroskamux

commit ca9b6b55e61a14c448078e8291dc0e9cf3c19dde
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Sun Oct 11 11:37:51 2015 +0100

    matroskamux: Create a TIME segment when creating streamable output
    
    Related to https://bugzilla.gnome.org/show_bug.cgi?id=754435 which
    does the same for flvmux.