From 1549aaba2749f9b8a2fc9e56c849c69a8107098f Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Tue, 16 Aug 2011 15:22:46 +0100 Subject: [PATCH 1/4] flacdec: warn if we see a variable block size where unsupported https://bugzilla.gnome.org/show_bug.cgi?id=650960 --- ext/flac/gstflacdec.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ext/flac/gstflacdec.c b/ext/flac/gstflacdec.c index 2e4085dd63..1f797ba8f0 100644 --- a/ext/flac/gstflacdec.c +++ b/ext/flac/gstflacdec.c @@ -408,8 +408,12 @@ gst_flac_dec_scan_got_frame (GstFlacDec * flacdec, guint8 * data, guint size, return FALSE; /* sync */ - if (data[0] != 0xFF || data[1] != 0xF8) + if (data[0] != 0xFF || (data[1] & 0xFC) != 0xF8) return FALSE; + if (data[1] & 1) { + GST_WARNING_OBJECT (flacdec, "Variable block size FLAC unsupported"); + return FALSE; + } bs = (data[2] & 0xF0) >> 8; /* blocksize marker */ sr = (data[2] & 0x0F); /* samplerate marker */ From 64beef46101b023ca4f052c769ce9b0280ca9dcc Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Tue, 16 Aug 2011 15:27:43 +0100 Subject: [PATCH 2/4] flacdec: fix bit twiddling Right shifting a 8 bit value by 8 bits is twice too much to get the high 4 bits. https://bugzilla.gnome.org/show_bug.cgi?id=650960 --- ext/flac/gstflacdec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/flac/gstflacdec.c b/ext/flac/gstflacdec.c index 1f797ba8f0..8fdd1091b4 100644 --- a/ext/flac/gstflacdec.c +++ b/ext/flac/gstflacdec.c @@ -415,9 +415,9 @@ gst_flac_dec_scan_got_frame (GstFlacDec * flacdec, guint8 * data, guint size, return FALSE; } - bs = (data[2] & 0xF0) >> 8; /* blocksize marker */ + bs = (data[2] & 0xF0) >> 4; /* blocksize marker */ sr = (data[2] & 0x0F); /* samplerate marker */ - ca = (data[3] & 0xF0) >> 8; /* channel assignment */ + ca = (data[3] & 0xF0) >> 4; /* channel assignment */ ss = (data[3] & 0x0F) >> 1; /* sample size marker */ pb = (data[3] & 0x01); /* padding bit */ From e09eb95a5fd46bcbf3088dbdff780a4adebb0a75 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Tue, 16 Aug 2011 15:32:07 +0100 Subject: [PATCH 3/4] flacdec: bail on reserved value Now that we look at the right bits, we can test against the reserved value as we do for other fields. https://bugzilla.gnome.org/show_bug.cgi?id=650960 --- ext/flac/gstflacdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/flac/gstflacdec.c b/ext/flac/gstflacdec.c index 8fdd1091b4..94f77899f3 100644 --- a/ext/flac/gstflacdec.c +++ b/ext/flac/gstflacdec.c @@ -424,7 +424,7 @@ gst_flac_dec_scan_got_frame (GstFlacDec * flacdec, guint8 * data, guint size, GST_LOG_OBJECT (flacdec, "got sync, bs=%x,sr=%x,ca=%x,ss=%x,pb=%x", bs, sr, ca, ss, pb); - if (sr == 0x0F || ca >= 0x0B || ss == 0x03 || ss == 0x07) { + if (bs == 0 || sr == 0x0F || ca >= 0x0B || ss == 0x03 || ss == 0x07) { return FALSE; } From 3e0134f51fe174a1c68422b55147b2f2a2b07373 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Tue, 16 Aug 2011 17:27:13 +0100 Subject: [PATCH 4/4] flacdec: avoid timestamp/offset tracking going out of sync The libFLAC API is callback based, and we must only call it to output data when we know we have enough input data. For this reason, a single processing step is done when receiving a buffer. However, if there were metadata buffers still pending, a step intended for the first audio frame might end up writing that leftover metadata. Since a single step is done per buffer, this will cause every buffer to be written one step late. This would add some latency (a bufferfull's worth), possibly lose a buffer when seeking or the like, and also cause timestamp and offset to be applied to the wrong buffer, as updates to the "current" segment last_stop (from incoming buffer timestamp) will be applied to an output buffer originating from the previous incoming buffer. This fixes the issue by ensuring that, upon receiving the first audio frame, processing is done till all metadata is processed, so the next "single step" done will be for the audio frame. After this, we should keep to 1 input buffer -> 1 output buffer and so avoid getting out of sync. https://bugzilla.gnome.org/show_bug.cgi?id=650960 --- ext/flac/gstflacdec.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ext/flac/gstflacdec.c b/ext/flac/gstflacdec.c index 94f77899f3..061fa811f7 100644 --- a/ext/flac/gstflacdec.c +++ b/ext/flac/gstflacdec.c @@ -1391,8 +1391,10 @@ gst_flac_dec_chain (GstPad * pad, GstBuffer * buf) dec = GST_FLAC_DEC (GST_PAD_PARENT (pad)); - GST_LOG_OBJECT (dec, "buffer with ts=%" GST_TIME_FORMAT ", end_offset=%" - G_GINT64_FORMAT ", size=%u", GST_TIME_ARGS (GST_BUFFER_TIMESTAMP (buf)), + GST_LOG_OBJECT (dec, + "buffer with ts=%" GST_TIME_FORMAT ", offset=%" G_GINT64_FORMAT + ", end_offset=%" G_GINT64_FORMAT ", size=%u", + GST_TIME_ARGS (GST_BUFFER_TIMESTAMP (buf)), GST_BUFFER_OFFSET (buf), GST_BUFFER_OFFSET_END (buf), GST_BUFFER_SIZE (buf)); if (dec->init) { @@ -1483,6 +1485,19 @@ gst_flac_dec_chain (GstPad * pad, GstBuffer * buf) /* framed - there should always be enough data to decode something */ GST_LOG_OBJECT (dec, "%u bytes available", gst_adapter_available (dec->adapter)); + if (G_UNLIKELY (!dec->got_headers)) { + /* The first time we get audio data, we know we got all the headers. + * We then loop until all the metadata is processed, then do an extra + * "process_single" step for the audio frame. */ + GST_DEBUG_OBJECT (dec, + "First audio frame, ensuring all metadata is processed"); + if (!FLAC__stream_decoder_process_until_end_of_metadata (dec->decoder)) { + GST_DEBUG_OBJECT (dec, "process_until_end_of_metadata failed"); + } + GST_DEBUG_OBJECT (dec, + "All metadata is now processed, reading to process audio data"); + dec->got_headers = TRUE; + } if (!FLAC__stream_decoder_process_single (dec->decoder)) { GST_DEBUG_OBJECT (dec, "process_single failed"); }