Commit graph

138 commits

Author SHA1 Message Date
Jennifer Berringer
3287f1cb3f flacparse: fix broken reordering of flac metadata
Each FLAC metadata block starts with a flag denoting whether it is the
last metadata block. The existing flacparse code moves any existing
VORBISCOMMENT block to immediately follow the STREAMINFO block without
changing any block's last-metadata-block flag. If no VORBISCOMMENT block
exists, it created one with the last-metadata-block flag set to true.
This results in gstflacdec sometimes giving bad headers to libflac when
trying to play perfectly valid FLAC files depending on the file's
metadata ordering. Depending on the contents of the other metadata
blocks, current versions of libflac may or may not return
FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER when given this broken
metadata. This is most noticeable with files that have a large cover art
image attached where VORBISCOMMENT is the very last metadata block with
no PADDING afterwards.

This patch changes that behavior so that:

1. For FLAC files that already have a VORBISCOMMENT block, the metadata
   order is preserved.
2. For FLAC files that do not have a VORBISCOMMENT block, the generated
   dummy VORBISCOMMENT is placed immediately after STREAMINFO and
   inherits the last-metadata-block flag from STREAMINFO.

https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/484
2020-03-03 08:03:32 +00:00
Thibault Saunier
0a6a62aa76 docs: Port all docstring to gtk-doc markdown 2019-05-13 10:24:40 -04:00
Jochen Henneberg
7824e87c5b flacparse: On sink caps change restart parser
Draining the parser is not enough here, on caps change we need to
reset it so it is ready to accept new caps.
2018-12-14 09:22:33 +00:00
Sebastian Dröge
336a97157c flacparse: Drain the parser when a CAPS event is received
After a CAPS event, in theory a new stream can start and it might start
with the FLAC headers again. We can't detect FLAC headers in the middle
of the stream, so we drain the parser to be able to detect either FLAC
headers after the CAPS event or the continuation of the previous stream.

This fixes for example

gst-launch-1.0 audiotestsrc num-buffers=200 ! flacenc ! c. \
    audiotestsrc num-buffers=200 freq=880 ! flacenc ! c. \
    concat name=c ! rtpgstpay ! udpsink host=127.0.0.1 port=5000

gst-launch-1.0 udpsrc multicast-group=127.0.0.1 port=5000 \
    caps=application/x-rtp,media=application,clock-rate=90000,encoding-name=X-GST ! \
    rtpgstdepay ! flacparse ! flacdec ! audioconvert ! pulsesin
2018-04-16 21:29:57 +03:00
Tim-Philipp Müller
d4c04cb079 Revert "flacparse: fix header rewriting being ignored"
This caused broken metadata and also looks a bit dodgy.
Revert until we can figure out a solution that works for
all cases and doesn't break anything.

This reverts commit adeee44b07.

https://bugzilla.gnome.org/show_bug.cgi?id=727802
https://bugzilla.gnome.org/show_bug.cgi?id=785558
2017-12-07 11:17:32 +00:00
Sebastian Dröge
b02350bd62 flacparse: Request at least the full header size when parsing headers
Otherwise baseparse will incrementally send us bigger buffers until the
full header size is reached, which is not only pointless but also means
that baseparse will reallocate and copy into a bigger buffer for every
input buffers. In pull mode that's done in 64kb increments, in push mode
usually in much smaller increments, causing a lot of overhead for
example when parsing high-quality coverart.
2017-11-29 17:28:25 +02:00
Vincent Penquerc'h
f35238794c flacparse: fix playback if sample number does not start at 0
This reverts commit 29b807685d, while
fixing the original breaking tests/check/pipelines/flacdec.
2017-03-17 14:11:44 +00:00
Vincent Penquerc'h
29b807685d Revert "flacparse: fix playback if sample number does not start at 0"
This breaks gst-validate on the build server (though not locally),
and a unit test, and I can't run unit tests right now for some
unrelated reason.

This reverts commit 0747b56f8e.
2017-03-17 11:30:04 +00:00
Vincent Penquerc'h
0747b56f8e flacparse: fix playback if sample number does not start at 0
https://bugzilla.gnome.org/show_bug.cgi?id=777738
2017-03-16 14:21:20 +00:00
Vincent Penquerc'h
adeee44b07 flacparse: fix header rewriting being ignored
https://bugzilla.gnome.org/show_bug.cgi?id=727802
2016-11-10 12:51:08 +00:00
Tim-Philipp Müller
0f41d0e75d Revert "flacparse: Add maximum bitrate tag"
This reverts commit c703ab69f5.

https://bugzilla.gnome.org/show_bug.cgi?id=769392
2016-08-18 12:02:01 +01:00
Carlos Rafael Giani
c703ab69f5 flacparse: Add maximum bitrate tag
https://bugzilla.gnome.org/show_bug.cgi?id=769392
2016-08-02 14:34:54 +03:00
Vineeth TM
1071309870 good: use new gst_element_class_add_static_pad_template()
https://bugzilla.gnome.org/show_bug.cgi?id=763076
2016-03-24 14:32:20 +02:00
Sebastian Dröge
66e9e4c202 Revert "flacparse: push tags in pre_push_frame"
This reverts commit 4065fcb80a.

flacparse should not push tags by itself, the base class is going to do that
while properly merging in upstream tags. It just didn't because of a bug in
the base class, which was hidden by this commit.

https://bugzilla.gnome.org/show_bug.cgi?id=763553
2016-03-13 10:33:13 +02:00
Luis de Bethencourt
4065fcb80a flacparse: push tags in pre_push_frame
Push a tag event before pre-roll if we have tags.

https://bugzilla.gnome.org/show_bug.cgi?id=762660
2016-03-01 19:23:02 +00:00
Reynaldo H. Verdejo Pinochet
0bb8000874 flacparse: demote warning on wrong reserved value to fixme
We are likely just parsing a backward-compatible stream we
don't fully support.
2016-01-10 22:54:12 -08:00
Tim-Philipp Müller
6dab3ece07 flacparse: don't map buffer multiple times when parsing 2016-01-07 16:24:09 +00:00
Reynaldo H. Verdejo Pinochet
ba094b50e1 flacparse: add debug msg on CRC mismatch while validating frame header 2015-12-31 16:04:15 -08:00
Reynaldo H. Verdejo Pinochet
6b7675e4a2 flacparse: drop unneeded braces at _parse_frame() exit
Additionally, drop redundant comment & line break
2015-12-31 16:04:15 -08:00
Reynaldo H. Verdejo Pinochet
b6ebad0997 flacparse: minor grammar correction 2015-12-31 16:04:15 -08:00
Reynaldo H. Verdejo Pinochet
5234c7c2bd flacparse: update URLs on pointers to online spec 2015-12-31 15:34:57 -08:00
Reynaldo H. Verdejo Pinochet
5f4317843c flacparse: make buffer DTS setting explicitly unconditional
We are setting it to PTS regardless of block_strategy
2015-12-31 14:40:15 -08:00
Reynaldo H. Verdejo Pinochet
2c14f2fff1 flacparse: add actual invalid block type to warning
For someone that read the spec is clear the only *invalid*
data block type is 127. For the rest, its useful information.

Additionally. values 7-126 are currently reserved by the
spec so the situation might change in the future.
2015-12-31 14:21:40 -08:00
Reynaldo H. Verdejo Pinochet
c43f84abf3 flacparse: use shift instead of mask & comp
We are only interested on the first bit of the first
byte of the metadata block header to figure out whether
is marked as the last one. The shift makes it quite
clearer.
2015-12-31 14:12:36 -08:00
Reynaldo H. Verdejo Pinochet
8a745837aa flacparse: warn on wishful parsing of weird headers
If we get anything from 7 to 126 as type when parsing
a metadata block header, we are likely dealing with a
FLAC stream version we don't fully understand. Issue
a warning if so.

Document function assumptions regarding the passed-on
type while at this.
2015-12-31 13:04:23 -08:00
Reynaldo H. Verdejo Pinochet
df6f0bc595 flacparse: show meaningful info on frame CRC check
As CRCs are calculated for the comparition already, we
might as well (cheaply) inform the user how the numbers
differ if a missmatched pair is found.

While at it:

Rephrase candidate-frame message to make more sense
2015-12-31 13:04:23 -08:00
Reynaldo H. Verdejo Pinochet
395afed566 flacparse: drop remaining trailing whitespace 2015-12-31 13:04:23 -08:00
Reynaldo H. Verdejo Pinochet
a086ee6192 flacparse: drop superflous else clauses 2015-12-31 13:04:23 -08:00
Reynaldo H. Verdejo Pinochet
7286aae6e5 flacparse: factor out buffer time and offset resetting
Avoids multiple occurrences of the same resetting pattern
2015-12-31 13:04:23 -08:00
Reynaldo H. Verdejo Pinochet
5bf1f1ec9c flacparse: move block handling by type out of _parse_frame() 2015-12-31 13:04:23 -08:00
Reynaldo H. Verdejo Pinochet
eb47176b7c flacparse: drop redundant return statement on _header_is_valid()
Fix the rather vague error message while at it.
2015-12-30 22:33:58 -08:00
Reynaldo H. Verdejo Pinochet
276fcc5916 flacparse: rework gst_flac_parse_frame_is_valid()
drop unnecessary nesting looking for end of frame
2015-12-30 21:43:55 -08:00
Reynaldo H. Verdejo Pinochet
90b62be301 flacparse: factor out context clearing routine 2015-12-30 21:43:45 -08:00
Dave Craig
328346ad21 audioparsers: Check for NULL return value of gst_pad_get_current_caps()
https://bugzilla.gnome.org/show_bug.cgi?id=759503
2015-12-16 10:12:44 +01:00
Tim-Philipp Müller
0d88f27108 flacparse: set both pts and dts so baseparse doesn't make up wrong dts after a seek
flac contains the sample offset in the frame header, so after a seek
without index flacparse will know the exact position we landed on and
timestamp buffers accordingly. It only set the pts though, which means
the baseparse-set dts which was set to the seek position prevails, and
since the seek was based on an estimate, there's likely a discrepancy
between where we wanted to land and where we did land, so from here on
that dts/pts difference will be maintained, with dts possibly multiple
seconds ahead of pts, which is just wrong. The easiest way to fix this
is to just set both pts and dts based on the sample offset, but perhaps
parsed audio should just not have dts set at all.

https://bugzilla.gnome.org/show_bug.cgi?id=752106
2015-09-06 16:36:44 +01:00
Tim-Philipp Müller
a042a98159 flacparse: use new baseparse API and fix tag handling
https://bugzilla.gnome.org/show_bug.cgi?id=679768
2015-08-16 16:33:55 +01:00
Thiago Santos
9523fb23ed audioparsers: enable accept-template flag
Do a quick check with the pad template caps as it is enough. Users
should have figured the appropriate full caps on a previous caps query

https://bugzilla.gnome.org/show_bug.cgi?id=753623
2015-08-14 13:42:27 -03:00
Vineeth TM
e44ce40455 flacparse: fix possible memory leak
when buffer is stored to seektable, and stop gets called due to
corrupt flac file, then the seektable is not being released

https://bugzilla.gnome.org/show_bug.cgi?id=751364
2015-06-23 10:17:53 +02:00
Guillaume Desmottes
2bd3685d04 flacparse: fix buffer leak when stored to seektable
Fix a leak with the
validate.file.playback.change_state_intensive.samples_multimedia_cx_flac_Yesterday_flac
scenario.

https://bugzilla.gnome.org/show_bug.cgi?id=749072
2015-05-08 11:11:40 +01:00
Sebastian Dröge
91a3d044f0 flacparse: Only calculate with durations != -1 2014-09-23 22:56:21 +03:00
Ognyan Tonchev
7cd335e9b9 flacparse: do not leak uid after parsing TOC event
https://bugzilla.gnome.org/show_bug.cgi?id=736739
2014-09-17 09:51:15 +03:00
Tim-Philipp Müller
615f6e55c1 flacparse: skip PICTURE headers without any image data
Fixes warning if the image length is 0.
2014-05-07 00:58:15 +01:00
Vincent Penquerc'h
937269d02e flacparse: remove dead code
The block_size == 0 was shortcut earlier, and the variable is not
modified in the meantime.

Coverity 206097
2014-04-16 17:44:50 +01:00
Sebastian Dröge
05e196cbb6 flacparse: Post AUDIO_CODEC tag
https://bugzilla.gnome.org/show_bug.cgi?id=720512
2013-12-16 10:03:06 +01:00
Wim Taymans
0d55724a2b audioparsers: don't leak template caps 2013-12-04 09:12:07 +01:00
Wim Taymans
e0a5c07e8d audioparsers: use ACCEPT_INTERSECT flag
The parser can accept input that is not completely specified. Use the
ACCEPT_INTERSECT flag on the sinkpad to tweak the acceptcaps function to
check for intersection only. This allows us to proxy downstream
constraints while still allowing non-subset caps as input.
We can then also remove the appended template caps workaround.
Make a unit-test to check the new feature.

This reverts commit 26040ee38c

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=705024
2013-12-03 22:26:44 +01:00
Wim Taymans
e3f393f7e6 audioparsers: remove fields from filter
We need to remove the fields from the filter when we can convert
between them.
2013-12-03 21:39:57 +01:00
Wim Taymans
e8313a1e70 audioparsers: refactor code to remove caps fields 2013-12-03 21:29:13 +01:00
Sebastian Dröge
b95ddd55cd flacparse: Make sure we have enough data to read image tags
Thanks to iputinei for reporting this on IRC.
2013-09-12 15:39:51 +02:00
Matej Knopp
349afc633a flacparse: cleanup on error after state change
https://bugzilla.gnome.org/show_bug.cgi?id=707229
2013-09-03 18:06:18 +02:00