mpg123: cleaned up comments, formatting, and logging lines

also replaced mpg123decoder->handle != NULL checks with asserts

https://bugzilla.gnome.org/show_bug.cgi?id=686595
This commit is contained in:
Carlos Rafael Giani 2012-10-24 00:21:45 +02:00 committed by Tim-Philipp Müller
parent d51a5e7aea
commit 08707937dd

View file

@ -28,8 +28,7 @@
GST_DEBUG_CATEGORY_STATIC (mpg123_debug);
#define GST_CAT_DEFAULT mpg123_debug
/*
* Omitted sample formats that mpg123 supports (or at least can support):
/* Omitted sample formats that mpg123 supports (or at least can support):
* - 8bit integer signed
* - 8bit integer unsigned
* - a-law
@ -46,10 +45,10 @@ GST_DEBUG_CATEGORY_STATIC (mpg123_debug);
* no way how to find out which one of them is actually used.
*
* However, in all known installations, "real" equals 32bit float, so that's
* what is used.
*/
* what is used. */
static GstStaticPadTemplate sink_template = GST_STATIC_PAD_TEMPLATE ("sink",
static GstStaticPadTemplate static_sink_template =
GST_STATIC_PAD_TEMPLATE ("sink",
GST_PAD_SINK,
GST_PAD_ALWAYS,
GST_STATIC_CAPS ("audio/mpeg, "
@ -66,9 +65,9 @@ static GstFlowReturn gst_mpg123_audio_dec_push_decoded_bytes (GstMpg123AudioDec
* mpg123_decoder, unsigned char const *decoded_bytes,
size_t const num_decoded_bytes);
static GstFlowReturn gst_mpg123_audio_dec_handle_frame (GstAudioDecoder * dec,
GstBuffer * buffer);
GstBuffer * input_buffer);
static gboolean gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec,
GstCaps * incoming_caps);
GstCaps * input_caps);
static void gst_mpg123_audio_dec_flush (GstAudioDecoder * dec, gboolean hard);
G_DEFINE_TYPE (GstMpg123AudioDec, gst_mpg123_audio_dec, GST_TYPE_AUDIO_DECODER);
@ -79,7 +78,7 @@ gst_mpg123_audio_dec_class_init (GstMpg123AudioDecClass * klass)
GObjectClass *object_class;
GstAudioDecoderClass *base_class;
GstElementClass *element_class;
GstPadTemplate *src_template;
GstPadTemplate *src_template, *sink_template;
int error;
GST_DEBUG_CATEGORY_INIT (mpg123_debug, "mpg123", 0, "mpg123 mp3 decoder");
@ -96,15 +95,14 @@ gst_mpg123_audio_dec_class_init (GstMpg123AudioDecClass * klass)
"Decodes mp3 streams using the mpg123 library",
"Carlos Rafael Giani <dv@pseudoterminal.org>");
/*
Not using static pad template for srccaps, since the comma-separated list of formats needs to be
created depending on whatever mpg123 supports
*/
/* Not using static pad template for srccaps, since the comma-separated list
* of formats needs to be created depending on whatever mpg123 supports */
{
const int *format_list;
const long *rates_list;
size_t num, i;
GString *s;
GstCaps *src_template_caps;
s = g_string_new ("audio/x-raw, ");
@ -157,14 +155,16 @@ gst_mpg123_audio_dec_class_init (GstMpg123AudioDecClass * klass)
g_string_append (s, "channels = (int) [ 1, 2 ], ");
g_string_append (s, "layout = (string) interleaved");
src_template_caps = gst_caps_from_string (s->str);
src_template = gst_pad_template_new ("src", GST_PAD_SRC, GST_PAD_ALWAYS,
gst_caps_from_string (s->str));
src_template_caps);
g_string_free (s, TRUE);
}
gst_element_class_add_pad_template (element_class,
gst_static_pad_template_get (&sink_template));
sink_template = gst_static_pad_template_get (&static_sink_template);
gst_element_class_add_pad_template (element_class, sink_template);
gst_element_class_add_pad_template (element_class, src_template);
base_class->start = GST_DEBUG_FUNCPTR (gst_mpg123_audio_dec_start);
@ -179,7 +179,7 @@ gst_mpg123_audio_dec_class_init (GstMpg123AudioDecClass * klass)
GST_ERROR ("Could not initialize mpg123 library: %s",
mpg123_plain_strerror (error));
else
GST_TRACE ("mpg123 library initialized");
GST_INFO ("mpg123 library initialized");
}
@ -214,30 +214,34 @@ gst_mpg123_audio_dec_start (GstAudioDecoder * dec)
mpg123_decoder->has_next_audioinfo = FALSE;
mpg123_decoder->frame_offset = 0;
/*
Initially, the mpg123 handle comes with a set of default formats supported. This clears this set.
This is necessary, since only one format shall be supported (see set_format for more).
*/
/* Initially, the mpg123 handle comes with a set of default formats
* supported. This clears this set. This is necessary, since only one
* format shall be supported (see set_format for more). */
mpg123_format_none (mpg123_decoder->handle);
mpg123_param (mpg123_decoder->handle, MPG123_REMOVE_FLAGS, MPG123_GAPLESS, 0); /* Built-in mpg123 support for gapless decoding is disabled for now, since it does not work well with seeking */
mpg123_param (mpg123_decoder->handle, MPG123_ADD_FLAGS, MPG123_SEEKBUFFER, 0); /* Tells mpg123 to use a small read-ahead buffer for better MPEG sync; essential for MP3 radio streams */
mpg123_param (mpg123_decoder->handle, MPG123_RESYNC_LIMIT, -1, 0); /* Sets the resync limit to the end of the stream (e.g. don't give up prematurely) */
/* Built-in mpg123 support for gapless decoding is disabled for now,
* since it does not work well with seeking */
mpg123_param (mpg123_decoder->handle, MPG123_REMOVE_FLAGS, MPG123_GAPLESS, 0);
/* Tells mpg123 to use a small read-ahead buffer for better MPEG sync;
* essential for MP3 radio streams */
mpg123_param (mpg123_decoder->handle, MPG123_ADD_FLAGS, MPG123_SEEKBUFFER, 0);
/* Sets the resync limit to the end of the stream (otherwise mpg123 may give
* up on decoding prematurely, especially with mp3 web radios) */
mpg123_param (mpg123_decoder->handle, MPG123_RESYNC_LIMIT, -1, 0);
/* Open in feed mode (= encoded data is fed manually into the handle). */
error = mpg123_open_feed (mpg123_decoder->handle);
if (G_UNLIKELY (error != MPG123_OK)) {
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL),
("Error opening mpg123 feed: %s", mpg123_plain_strerror (error)));
GST_ELEMENT_ERROR (dec, LIBRARY, INIT, (NULL),
("%s", mpg123_strerror (mpg123_decoder->handle)));
mpg123_close (mpg123_decoder->handle);
mpg123_delete (mpg123_decoder->handle);
mpg123_decoder->handle = NULL;
return FALSE;
}
GST_DEBUG_OBJECT (dec, "mpg123 decoder started");
GST_INFO_OBJECT (dec, "mpg123 decoder started");
return TRUE;
}
@ -254,7 +258,7 @@ gst_mpg123_audio_dec_stop (GstAudioDecoder * dec)
mpg123_decoder->handle = NULL;
}
GST_DEBUG_OBJECT (dec, "mpg123 decoder stopped");
GST_INFO_OBJECT (dec, "mpg123 decoder stopped");
return TRUE;
}
@ -265,39 +269,36 @@ gst_mpg123_audio_dec_push_decoded_bytes (GstMpg123AudioDec * mpg123_decoder,
unsigned char const *decoded_bytes, size_t const num_decoded_bytes)
{
GstBuffer *output_buffer;
GstFlowReturn alloc_error;
GstAudioDecoder *dec;
output_buffer = NULL;
dec = GST_AUDIO_DECODER (mpg123_decoder);
if ((num_decoded_bytes == 0) || (decoded_bytes == NULL)) {
/* This occurs in the first few frames, which do not carry data; once MPG123_AUDIO_DEC_NEW_FORMAT is received, the empty frames stop occurring */
GST_TRACE_OBJECT (mpg123_decoder,
"Nothing was decoded -> no output buffer to push");
/* This occurs in the first few frames, which do not carry data; once
* MPG123_AUDIO_DEC_NEW_FORMAT is received, the empty frames stop occurring */
GST_DEBUG_OBJECT (mpg123_decoder,
"cannot decode yet, need more data -> no output buffer to push");
return GST_FLOW_OK;
}
output_buffer = gst_buffer_new_allocate (NULL, num_decoded_bytes, NULL);
alloc_error = (output_buffer == NULL) ? GST_FLOW_ERROR : GST_FLOW_OK;
if (alloc_error != GST_FLOW_OK) {
/* This is necessary to advance playback in time, even when nothing was decoded. */
if (output_buffer == NULL) {
/* This is necessary to advance playback in time,
* even when nothing was decoded. */
return gst_audio_decoder_finish_frame (dec, NULL, 1);
} else {
GstMapInfo info;
if (gst_buffer_map (output_buffer, &info, GST_MAP_WRITE)) {
if (info.size != num_decoded_bytes)
GST_ERROR_OBJECT (mpg123_decoder,
"Mapped memory region has size %u instead of expected size %u",
info.size, num_decoded_bytes);
else
memcpy (info.data, decoded_bytes, num_decoded_bytes);
gst_buffer_unmap (output_buffer, &info);
} else
GST_ERROR_OBJECT (mpg123_decoder, "Could not map buffer");
} else {
GST_ERROR_OBJECT (mpg123_decoder, "gst_buffer_map() returned NULL");
gst_buffer_unref (output_buffer);
output_buffer = NULL;
}
return gst_audio_decoder_finish_frame (dec, output_buffer, 1);
}
@ -305,78 +306,66 @@ gst_mpg123_audio_dec_push_decoded_bytes (GstMpg123AudioDec * mpg123_decoder,
static GstFlowReturn
gst_mpg123_audio_dec_handle_frame (GstAudioDecoder * dec, GstBuffer * buffer)
gst_mpg123_audio_dec_handle_frame (GstAudioDecoder * dec,
GstBuffer * input_buffer)
{
GstMpg123AudioDec *mpg123_decoder;
int decode_error;
unsigned char *decoded_bytes;
size_t num_decoded_bytes;
GstFlowReturn retval;
if (G_UNLIKELY (!buffer))
if (G_UNLIKELY (input_buffer == NULL))
return GST_FLOW_OK;
mpg123_decoder = GST_MPG123_AUDIO_DEC (dec);
if (G_UNLIKELY (mpg123_decoder->handle == NULL)) {
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL),
("mpg123 handle is NULL"));
return GST_FLOW_ERROR;
}
g_assert (mpg123_decoder->handle != NULL);
/* The actual decoding */
{
unsigned char const *inmemory;
size_t inmemsize;
GstMemory *memory;
GstMapInfo info;
memory = gst_buffer_get_all_memory (buffer);
if (memory == NULL)
return GST_FLOW_ERROR;
if (!gst_memory_map (memory, &info, GST_MAP_READ)) {
gst_memory_unref (memory);
/* feed input data */
if (gst_buffer_map (input_buffer, &info, GST_MAP_READ)) {
mpg123_feed (mpg123_decoder->handle, info.data, info.size);
gst_buffer_unmap (input_buffer, &info);
} else {
GST_ERROR_OBJECT (mpg123_decoder, "gst_memory_map() failed");
return GST_FLOW_ERROR;
}
inmemory = info.data;
inmemsize = info.size;
mpg123_feed (mpg123_decoder->handle, inmemory, inmemsize);
/* Try to decode a frame */
decoded_bytes = NULL;
num_decoded_bytes = 0;
decode_error = mpg123_decode_frame (mpg123_decoder->handle,
&mpg123_decoder->frame_offset, &decoded_bytes, &num_decoded_bytes);
gst_memory_unmap (memory, &info);
gst_memory_unref (memory);
}
retval = GST_FLOW_OK;
switch (decode_error) {
case MPG123_NEW_FORMAT:
/*
As mentioned in gst_mpg123_audio_dec_set_format(), the next audioinfo is not set immediately;
instead, the code waits for mpg123 to take note of the new format, and then sets the audioinfo
This fixes glitches with mp3s containing several format headers (for example, first half using 44.1kHz, second half 32 kHz)
*/
/* As mentioned in gst_mpg123_audio_dec_set_format(), the next audioinfo
* is not set immediately; instead, the code waits for mpg123 to take
* note of the new format, and then sets the audioinfo. This fixes glitches
* with mp3s containing several format headers (for example, first half
* using 44.1kHz, second half 32 kHz) */
GST_DEBUG_OBJECT (dec,
GST_LOG_OBJECT (dec,
"mpg123 reported a new format -> setting next srccaps");
gst_mpg123_audio_dec_push_decoded_bytes (mpg123_decoder, decoded_bytes,
num_decoded_bytes);
/*
If there is a next audioinfo, use it, then set has_next_audioinfo to FALSE, to make sure
gst_audio_decoder_set_output_format() isn't called again until set_format is called by the base class
*/
/* If there is a next audioinfo, use it, then set has_next_audioinfo to
* FALSE, to make sure gst_audio_decoder_set_output_format() isn't called
* again until set_format is called by the base class */
if (mpg123_decoder->has_next_audioinfo) {
if (!gst_audio_decoder_set_output_format (dec,
&(mpg123_decoder->next_audioinfo))) {
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL),
("Unable to set output format"));
GST_WARNING_OBJECT (dec, "Unable to set output format");
retval = GST_FLOW_NOT_NEGOTIATED;
}
mpg123_decoder->has_next_audioinfo = FALSE;
}
@ -385,61 +374,91 @@ gst_mpg123_audio_dec_handle_frame (GstAudioDecoder * dec, GstBuffer * buffer)
case MPG123_NEED_MORE:
case MPG123_OK:
return gst_mpg123_audio_dec_push_decoded_bytes (mpg123_decoder,
retval = gst_mpg123_audio_dec_push_decoded_bytes (mpg123_decoder,
decoded_bytes, num_decoded_bytes);
break;
/* If this happens, then the upstream parser somehow missed the ending of the bitstream */
case MPG123_DONE:
GST_DEBUG_OBJECT (dec, "mpg123 is done decoding");
/* If this happens, then the upstream parser somehow missed the ending
* of the bitstream */
GST_LOG_OBJECT (dec, "mpg123 is done decoding");
gst_mpg123_audio_dec_push_decoded_bytes (mpg123_decoder, decoded_bytes,
num_decoded_bytes);
return GST_FLOW_EOS;
retval = GST_FLOW_EOS;
break;
/* Anything else is considered an error */
default:
{
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL), ("Decoding error: %s",
mpg123_plain_strerror (decode_error)));
return GST_FLOW_ERROR;
/* Anything else is considered an error */
int errcode;
switch (decode_error) {
case MPG123_ERR:
errcode = mpg123_errcode (mpg123_decoder->handle);
break;
default:
errcode = decode_error;
}
switch (errcode) {
case MPG123_BAD_OUTFORMAT:{
GstCaps *input_caps =
gst_pad_get_current_caps (GST_AUDIO_DECODER_SINK_PAD (dec));
GST_ELEMENT_ERROR (dec, STREAM, FORMAT, (NULL),
("Output sample format could not be used when trying to decode frame. "
"This is typically caused when the input caps (often the sample "
"rate) do not match the actual format of the audio data. "
"Input caps: %" GST_PTR_FORMAT, input_caps)
);
gst_caps_unref (input_caps);
break;
}
default:{
char const *errmsg = mpg123_plain_strerror (errcode);
GST_ERROR_OBJECT (dec, "Reported error: %s", errmsg);
}
}
return GST_FLOW_OK;
retval = GST_FLOW_ERROR;
}
}
return retval;
}
static gboolean
gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec, GstCaps * incoming_caps)
gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec, GstCaps * input_caps)
{
/*
Using the parsed information upstream, and the list of allowed caps downstream, this code
tries to find a suitable audio info. It is important to keep in mind that the rate and number of channels
should never deviate from the one the bitstream has, otherwise mpg123 has to mix channels and/or
resample (and as its docs say, its internal resampler is very crude). The sample format, however,
can be chosen freely, because the MPEG specs do not mandate any special format.
Therefore, rate and number of channels are taken from upstream (which parsed the MPEG frames, so
the incoming_caps contain exactly the rate and number of channels the bitstream actually has), while
the sample format is chosen by trying out all caps that are allowed by downstream. This way, the output
is adjusted to what the downstream prefers.
Also, the new output audio info is not set immediately. Instead, it is considered the "next audioinfo".
The code waits for mpg123 to notice the new format (= when mpg123_decode_frame() returns MPG123_AUDIO_DEC_NEW_FORMAT),
and then sets the next audioinfo. Otherwise, the next audioinfo is set too soon, which may cause problems with
mp3s containing several format headers. One example would be an mp3 with the first 30 seconds using 44.1 kHz,
then the next 30 seconds using 32 kHz. Rare, but possible.
STEPS:
1. get rate and channels from incoming_caps
2. get allowed caps from src pad
3. for each structure in allowed caps:
3.1. take format
3.2. if the combination of format with rate and channels is unsupported by mpg123, go to (3),
or exit with error if there are no more structures to try
3.3. create next audioinfo out of rate,channels,format, and exit
*/
/* Using the parsed information upstream, and the list of allowed caps
* downstream, this code tries to find a suitable audio info. It is important
* to keep in mind that the rate and number of channels should never deviate
* from the one the bitstream has, otherwise mpg123 has to mix channels and/or
* resample (and as its docs say, its internal resampler is very crude). The
* sample format, however, can be chosen freely, because the MPEG specs do not
* mandate any special format. Therefore, rate and number of channels are taken
* from upstream (which parsed the MPEG frames, so the input_caps contain
* exactly the rate and number of channels the bitstream actually has), while
* the sample format is chosen by trying out all caps that are allowed by
* downstream. This way, the output is adjusted to what the downstream prefers.
*
* Also, the new output audio info is not set immediately. Instead, it is
* considered the "next audioinfo". The code waits for mpg123 to notice the new
* format (= when mpg123_decode_frame() returns MPG123_AUDIO_DEC_NEW_FORMAT),
* and then sets the next audioinfo. Otherwise, the next audioinfo is set too
* soon, which may cause problems with mp3s containing several format headers.
* One example would be an mp3 with the first 30 seconds using 44.1 kHz, then
* the next 30 seconds using 32 kHz. Rare, but possible.
*
* STEPS:
*
* 1. get rate and channels from input_caps
* 2. get allowed caps from src pad
* 3. for each structure in allowed caps:
* 3.1. take format
* 3.2. if the combination of format with rate and channels is unsupported by
* mpg123, go to (3), or exit with error if there are no more structures
* to try
* 3.3. create next audioinfo out of rate,channels,format, and exit
*/
int rate, channels;
@ -450,30 +469,26 @@ gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec, GstCaps * incoming_caps)
mpg123_decoder = GST_MPG123_AUDIO_DEC (dec);
if (G_UNLIKELY (mpg123_decoder->handle == NULL)) {
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL),
("mpg123 handle is NULL"));
return FALSE;
}
g_assert (mpg123_decoder->handle != NULL);
mpg123_decoder->has_next_audioinfo = FALSE;
/* Get rate and channels from incoming_caps */
/* Get rate and channels from input_caps */
{
GstStructure *structure;
gboolean err = FALSE;
/* Only the first structure is used (multiple incoming structures don't make sense */
structure = gst_caps_get_structure (incoming_caps, 0);
/* Only the first structure is used (multiple
* input caps structures don't make sense */
structure = gst_caps_get_structure (input_caps, 0);
if (!gst_structure_get_int (structure, "rate", &rate)) {
err = TRUE;
GST_ERROR_OBJECT (dec, "Incoming caps do not have a rate value");
GST_ERROR_OBJECT (dec, "Input caps do not have a rate value");
}
if (!gst_structure_get_int (structure, "channels", &channels)) {
err = TRUE;
GST_ERROR_OBJECT (dec, "Incoming caps do not have a channel value");
GST_ERROR_OBJECT (dec, "Input caps do not have a channel value");
}
if (err)
@ -485,8 +500,6 @@ gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec, GstCaps * incoming_caps)
GstCaps *allowed_srccaps_unnorm =
gst_pad_get_allowed_caps (GST_AUDIO_DECODER_SRC_PAD (dec));
allowed_srccaps = gst_caps_normalize (allowed_srccaps_unnorm);
/* TODO: this causes errors with 1.0 - perhaps a bug? */
/*gst_caps_unref(allowed_srccaps_unnorm); */
}
/* Go through all allowed caps, pick the first one that matches */
@ -549,7 +562,7 @@ gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec, GstCaps * incoming_caps)
GST_DEBUG_OBJECT (dec,
"mpg123 cannot use caps %" GST_PTR_FORMAT
" because mpg123_format() failed: %s", structure,
mpg123_plain_strerror (err));
mpg123_strerror (mpg123_decoder->handle));
continue;
}
}
@ -557,7 +570,7 @@ gst_mpg123_audio_dec_set_format (GstAudioDecoder * dec, GstCaps * incoming_caps)
gst_audio_info_init (&(mpg123_decoder->next_audioinfo));
gst_audio_info_set_format (&(mpg123_decoder->next_audioinfo), format, rate,
channels, NULL);
GST_DEBUG_OBJECT (dec, "The next audio format is: %s, %u Hz, %u channels",
GST_LOG_OBJECT (dec, "The next audio format is: %s, %u Hz, %u channels",
format_str, rate, channels);
mpg123_decoder->has_next_audioinfo = TRUE;
@ -580,25 +593,20 @@ gst_mpg123_audio_dec_flush (GstAudioDecoder * dec, gboolean hard)
hard = hard;
GST_DEBUG_OBJECT (dec, "Flushing decoder");
GST_LOG_OBJECT (dec, "Flushing decoder");
mpg123_decoder = GST_MPG123_AUDIO_DEC (dec);
if (G_UNLIKELY (mpg123_decoder->handle == NULL)) {
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL),
("mpg123 handle is NULL"));
return;
}
g_assert (mpg123_decoder->handle != NULL);
/* Flush by reopening the feed */
mpg123_close (mpg123_decoder->handle);
error = mpg123_open_feed (mpg123_decoder->handle);
if (G_UNLIKELY (error != MPG123_OK)) {
GstElement *element = GST_ELEMENT (dec);
GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL),
("Error reopening mpg123 feed: %s", mpg123_plain_strerror (error)));
GST_ELEMENT_ERROR (dec, LIBRARY, INIT, (NULL),
("Error while reopening mpg123 feed: %s",
mpg123_plain_strerror (error)));
mpg123_close (mpg123_decoder->handle);
mpg123_delete (mpg123_decoder->handle);
mpg123_decoder->handle = NULL;
@ -606,10 +614,10 @@ gst_mpg123_audio_dec_flush (GstAudioDecoder * dec, gboolean hard)
mpg123_decoder->has_next_audioinfo = FALSE;
/*
opening/closing feeds do not affect the format defined by the mpg123_format() call that was made in gst_mpg123_audio_dec_set_format(),
and since the up/downstream caps are not expected to change here, no mpg123_format() calls are done
*/
/* opening/closing feeds do not affect the format defined by the
* mpg123_format() call that was made in gst_mpg123_audio_dec_set_format(),
* and since the up/downstream caps are not expected to change here, no
* mpg123_format() calls are done */
}
static gboolean