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 777738 - flacparse: fix FLAC streaming for non-0 start sample
flacparse: fix FLAC streaming for non-0 start sample
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.10.2
Other Linux
: Normal normal
: 1.11.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-01-25 12:38 UTC by Christoph Reiter (lazka)
Modified: 2017-03-17 16:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
flacparse: fix playback if sample number does not start at 0 (3.44 KB, patch)
2017-03-16 13:56 UTC, Vincent Penquerc'h
none Details | Review
fix overeager selection of first sample number (970 bytes, patch)
2017-03-17 13:46 UTC, Vincent Penquerc'h
none Details | Review
flacparse: fix playback if sample number does not start at 0 (3.50 KB, patch)
2017-03-17 14:12 UTC, Vincent Penquerc'h
none Details | Review

Description Christoph Reiter (lazka) 2017-01-25 12:38:39 UTC
The stream https://chiru.no:8081/stream.flac does not work with GStreamer, but works with VLC or MPV.

1) gst-play-1.0 https://chiru.no:8081/stream.flac
2) Buffers to 100%

Expected: starts playing
Actual: nothing happens

Using 1.10.2
Comment 1 Sebastian Dröge (slomo) 2017-01-25 12:48:29 UTC
Confirmed, also with master and when playing locally the stream
Comment 2 Sebastian Dröge (slomo) 2017-01-25 13:01:52 UTC
Problem here is that the first PTS is many hours in the future and not starting at 0
Comment 3 Vincent Penquerc'h 2017-03-16 13:56:20 UTC
Created attachment 348086 [details] [review]
flacparse: fix playback if sample number does not start at 0
Comment 4 Sebastian Dröge (slomo) 2017-03-16 14:03:53 UTC
Comment on attachment 348086 [details] [review]
flacparse: fix playback if sample number does not start at 0

Looks good to me but an even nicer solution would of course be to adjust the segment but keep timestamps as-is.
Comment 5 Vincent Penquerc'h 2017-03-16 14:11:57 UTC
Yes, though none of the audio parsers there made any segment event, so I did not go there. I'm guessing there's no particular reason they don't then ?
Comment 6 Vincent Penquerc'h 2017-03-16 14:25:57 UTC
commit 0747b56f8e7f4731d67f8d13a4bdc453dde0fdf7
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Thu Mar 16 13:54:54 2017 +0000

    flacparse: fix playback if sample number does not start at 0
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777738
Comment 7 Sebastian Dröge (slomo) 2017-03-16 14:31:45 UTC
(In reply to Vincent Penquerc'h from comment #5)
> Yes, though none of the audio parsers there made any segment event, so I did
> not go there. I'm guessing there's no particular reason they don't then ?

It's tricky because of the base class managing the segment
Comment 9 Vincent Penquerc'h 2017-03-16 16:54:37 UTC
The flac file used in that command is a (broken) symlink in the current gst-integration-testsuites repo. How do I get/pull the actual file ?
Comment 10 Vincent Penquerc'h 2017-03-16 16:57:42 UTC
nvm, I found how.
Comment 11 Vincent Penquerc'h 2017-03-16 16:59:45 UTC
Works here though, repeatedly:

$ cat ./run-flac-validate.sh 
/home/v/src/gst-devtools/validate/tools/gst-validate-1.0-debug playbin uri=file:///home/v/src/gst-integration-testsuites/medias/defaults/flac/samples.multimedia.cx_flac_Yesterday.flac audio-sink=fakesink sync=true video-sink=fakesink sync=true --set-media-info /home/v/src/gst-integration-testsuites/medias/defaults/flac/samples.multimedia.cx_flac_Yesterday.flac.media_info
$ ./run-flac-validate.sh 
Starting pipeline
Pipeline started
     issue : EOS events that are part of the same pipeline 'operation' should have the same seqnum
             Detected on <flacparse0:src>
             Detected on <flacdec0:sink>
             Detected on <flacdec0:src>
             Detected on <inputselector0:sink_0>
             Detected on <inputselector0:src>
             Detected on <audiotee:sink>
             Detected on <audiotee:src_0>
             Detected on <streamsynchronizer0:sink_0>
             Detected on <streamsynchronizer0:src_0>
             Detected on <aqueue:sink>
             Detected on <aqueue:src>
             Detected on <conv:sink>
             Detected on <conv:src>
             Detected on <resample:sink>
             Detected on <resample:src>
             Detected on <volume:sink>
             Detected on <volume:src>
             Detected on <fakesink0:sink>
             Description : when events/messages are created from another event/message, they should have their seqnums set to the original event/message seqnum

   warning : received the same caps twice
             Detected on <flacdec0:sink>

Issues found: 2

=======> Test PASSED (Return value: 0)
Comment 12 Vincent Penquerc'h 2017-03-16 18:02:09 UTC
Does it break for you (locally) ?
Comment 13 Sebastian Dröge (slomo) 2017-03-17 10:22:18 UTC
It also breaks a unit test: https://ci.gstreamer.net/job/GStreamer-master/lastBuild/testReport/junit/(root)/flacdec/test_decode_seek_partial/

And gst-validate consistently fails.
Comment 14 Vincent Penquerc'h 2017-03-17 11:31:51 UTC
I can't run unit tests, I'm getting a hang in gst_poll when loading the registry. I've reverted the FLAC patch for now.
Comment 15 Vincent Penquerc'h 2017-03-17 12:29:04 UTC
I can see the unit test failure now. Will fix.
Comment 16 Vincent Penquerc'h 2017-03-17 13:46:12 UTC
Created attachment 348177 [details] [review]
fix overeager selection of first sample number

This (plus reverting the previous revert) fixes the unit test without breaking the file playback.
Comment 17 Sebastian Dröge (slomo) 2017-03-17 13:54:47 UTC
Comment on attachment 348177 [details] [review]
fix overeager selection of first sample number

Makes sense. Now does it also fix gst-validate? :)

When merging, please squash this with the original patch... instead of reverting the revert. Less noise
Comment 18 Vincent Penquerc'h 2017-03-17 13:56:43 UTC
The gst-validate test passes here with this. But it also did before, so that doesn't tell much.
Comment 19 Vincent Penquerc'h 2017-03-17 14:12:51 UTC
Created attachment 348180 [details] [review]
flacparse: fix playback if sample number does not start at 0
Comment 20 Vincent Penquerc'h 2017-03-17 14:34:04 UTC
I don't think one can get the gst-validate on the build server to try a patch, right ? I'd have to push it first ?
Comment 21 Sebastian Dröge (slomo) 2017-03-17 14:42:23 UTC
Yes
Comment 22 Vincent Penquerc'h 2017-03-17 16:10:01 UTC
I think the gst-validate test passed:

https://ci.gstreamer.net/job/GStreamer-master-validate/lastStableBuild/
Comment 23 Sebastian Dröge (slomo) 2017-03-17 16:29:51 UTC
Yes, thanks!