From aaed93e1265959bb06c4759e68537788c9446681 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Fri, 24 Jul 2009 21:54:59 +0100 Subject: kate: use GST_ELEMENT_ERROR for error reporting See #525743. --- ext/kate/gstkatedec.c | 11 ++++++----- ext/kate/gstkateenc.c | 40 ++++++++++++++++++++++++++-------------- ext/kate/gstkateparse.c | 12 ++++++------ ext/kate/gstkatespu.c | 23 +++++++++++++++-------- ext/kate/gstkatetag.c | 2 -- ext/kate/gstkatetiger.c | 2 -- ext/kate/gstkateutil.c | 4 ++-- 7 files changed, 55 insertions(+), 39 deletions(-) (limited to 'ext/kate') diff --git a/ext/kate/gstkatedec.c b/ext/kate/gstkatedec.c index fe5ac51c..eb4bb2ba 100644 --- a/ext/kate/gstkatedec.c +++ b/ext/kate/gstkatedec.c @@ -74,8 +74,6 @@ * */ -/* FIXME: post appropriate GST_ELEMENT_ERROR when returning FLOW_ERROR */ - #ifdef HAVE_CONFIG_H #include "config.h" #endif @@ -298,7 +296,8 @@ gst_kate_dec_chain (GstPad * pad, GstBuffer * buf) gst_flow_get_name (rflow)); } } else { - GST_WARNING_OBJECT (kd, "failed to create buffer"); + GST_ELEMENT_ERROR (kd, STREAM, DECODE, (NULL), + ("Failed to create buffer")); rflow = GST_FLOW_ERROR; } } else { @@ -307,7 +306,8 @@ gst_kate_dec_chain (GstPad * pad, GstBuffer * buf) } g_free (escaped); } else { - GST_WARNING_OBJECT (kd, "failed to allocate string"); + GST_ELEMENT_ERROR (kd, STREAM, DECODE, (NULL), + ("Failed to allocate string")); rflow = GST_FLOW_ERROR; } @@ -329,7 +329,8 @@ gst_kate_dec_chain (GstPad * pad, GstBuffer * buf) gst_flow_get_name (rflow)); } } else { - GST_WARNING_OBJECT (kd, "failed to create SPU from paletted bitmap"); + GST_ELEMENT_ERROR (kd, STREAM, DECODE, (NULL), + ("failed to create SPU from paletted bitmap")); rflow = GST_FLOW_ERROR; } } diff --git a/ext/kate/gstkateenc.c b/ext/kate/gstkateenc.c index 80709eb3..fd412d34 100644 --- a/ext/kate/gstkateenc.c +++ b/ext/kate/gstkateenc.c @@ -68,7 +68,6 @@ * */ -/* FIXME: post appropriate GST_ELEMENT_ERROR when returning FLOW_ERROR */ /* FIXME: should we automatically pick up the language code from the * upstream event tags if none was set via the property? */ @@ -457,7 +456,8 @@ gst_kate_enc_push_and_free_kate_packet (GstKateEnc * ke, kate_packet * kp, buffer = gst_kate_enc_create_buffer (ke, kp, granpos, timestamp, duration, header); if (G_UNLIKELY (!buffer)) { - GST_WARNING_OBJECT (ke, "Failed to create buffer, %u bytes", kp->nbytes); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Failed to create buffer, %u bytes", kp->nbytes)); kate_packet_clear (kp); return GST_FLOW_ERROR; } @@ -561,7 +561,8 @@ gst_kate_enc_send_headers (GstKateEnc * ke) GST_LOG_OBJECT (ke, "Last header encoded"); break; } else { - GST_LOG_OBJECT (ke, "Error encoding header: %d", ret); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("kate_encode_headers: %d", ret)); rflow = GST_FLOW_ERROR; break; } @@ -634,7 +635,8 @@ gst_kate_enc_chain_push_packet (GstKateEnc * ke, kate_packet * kp, granpos = kate_encode_get_granule (&ke->k); if (G_UNLIKELY (granpos < 0)) { - GST_WARNING_OBJECT (ke, "Negative granpos for packet"); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Negative granpos for packet")); kate_packet_clear (kp); return GST_FLOW_ERROR; } @@ -685,6 +687,8 @@ gst_kate_enc_flush_waiting (GstKateEnc * ke, GstClockTime now) ret = kate_encode_text (&ke->k, t0, t1, "", 0, &kp); if (G_UNLIKELY (ret < 0)) { + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("kate_encode_text: %d", ret)); rflow = GST_FLOW_ERROR; } else { rflow = @@ -747,7 +751,7 @@ gst_kate_enc_chain_spu (GstKateEnc * ke, GstBuffer * buf) g_free (kbitmap); if (kpalette) g_free (kpalette); - GST_ERROR_OBJECT (ke, "Out of memory"); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), ("Out of memory")); return GST_FLOW_ERROR; } @@ -802,17 +806,20 @@ gst_kate_enc_chain_spu (GstKateEnc * ke, GstBuffer * buf) kbitmap->width, kbitmap->height, GST_BUFFER_SIZE (buf), t0, t1); ret = kate_encode_set_region (&ke->k, kregion); if (G_UNLIKELY (ret < 0)) { - GST_WARNING_OBJECT (ke, "Failed to set event region (%d)", ret); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("kate_encode_set_region: %d", ret)); rflow = GST_FLOW_ERROR; } else { ret = kate_encode_set_palette (&ke->k, kpalette); if (G_UNLIKELY (ret < 0)) { - GST_WARNING_OBJECT (ke, "Failed to set event palette (%d)", ret); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("kate_encode_set_palette: %d", ret)); rflow = GST_FLOW_ERROR; } else { ret = kate_encode_set_bitmap (&ke->k, kbitmap); if (G_UNLIKELY (ret < 0)) { - GST_WARNING_OBJECT (ke, "Failed to set event bitmap (%d)", ret); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("kate_encode_set_bitmap: %d", ret)); rflow = GST_FLOW_ERROR; } else { /* Some SPUs have no hide time - so I'm going to delay the encoding of the packet @@ -833,8 +840,8 @@ gst_kate_enc_chain_spu (GstKateEnc * ke, GstBuffer * buf) } else { ret = kate_encode_text (&ke->k, t0, t1, "", 0, &kp); if (G_UNLIKELY (ret < 0)) { - GST_WARNING_OBJECT (ke, - "Failed to encode empty text for SPU buffer (%d)", ret); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Failed to encode empty text for SPU buffer: %d", ret)); rflow = GST_FLOW_ERROR; } else { rflow = @@ -875,7 +882,8 @@ gst_kate_enc_chain_text (GstKateEnc * ke, GstBuffer * buf, } if (G_UNLIKELY (ret < 0)) { - GST_WARNING_OBJECT (ke, "Failed to set markup type (%d)", ret); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("kate_encode_set_markup_type: %d", ret)); rflow = GST_FLOW_ERROR; } else { const char *text = (const char *) GST_BUFFER_DATA (buf); @@ -888,13 +896,17 @@ gst_kate_enc_chain_text (GstKateEnc * ke, GstBuffer * buf, GST_BUFFER_SIZE (buf), t0, t1); ret = kate_encode_text (&ke->k, t0, t1, text, text_len, &kp); if (G_UNLIKELY (ret < 0)) { + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Failed to encode text: %d", ret)); rflow = GST_FLOW_ERROR; } else { rflow = gst_kate_enc_chain_push_packet (ke, &kp, start, stop - start + 1); } } else { - GST_WARNING_OBJECT (ke, "No text in text packet"); + /* FIXME: this should not be an error, we should ignore it and move on */ + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("no text in text packet")); rflow = GST_FLOW_ERROR; } } @@ -918,8 +930,8 @@ gst_kate_enc_chain (GstPad * pad, GstBuffer * buf) /* get the type of the data we're being sent */ caps = GST_PAD_CAPS (pad); if (G_UNLIKELY (caps == NULL)) { - GST_ERROR_OBJECT (ke, ": Could not get caps of pad"); - rflow = GST_FLOW_ERROR; + GST_WARNING_OBJECT (ke, "No input caps set"); + rflow = GST_FLOW_NOT_NEGOTIATED; } else { const GstStructure *structure = gst_caps_get_structure (caps, 0); if (structure) diff --git a/ext/kate/gstkateparse.c b/ext/kate/gstkateparse.c index 89d17514..e9bcd5ae 100644 --- a/ext/kate/gstkateparse.c +++ b/ext/kate/gstkateparse.c @@ -58,8 +58,6 @@ * */ -/* FIXME: post appropriate GST_ELEMENT_ERROR when returning FLOW_ERROR */ - #ifdef HAVE_CONFIG_H # include "config.h" #endif @@ -174,7 +172,8 @@ gst_kate_parse_push_headers (GstKateParse * parse) gst_pad_get_negotiated_caps (parse->sinkpad), parse->streamheader); if (G_UNLIKELY (!caps)) { - GST_ERROR_OBJECT (parse, "Failed to set headers on caps"); + GST_ELEMENT_ERROR (parse, STREAM, DECODE, (NULL), + ("Failed to set headers on caps")); return GST_FLOW_ERROR; } @@ -182,8 +181,8 @@ gst_kate_parse_push_headers (GstKateParse * parse) res = gst_pad_set_caps (parse->srcpad, caps); gst_caps_unref (caps); if (G_UNLIKELY (!res)) { - GST_WARNING_OBJECT (parse, "Failed to set pad caps"); - return GST_FLOW_ERROR; + GST_WARNING_OBJECT (parse->srcpad, "Failed to set caps on source pad"); + return GST_FLOW_NOT_NEGOTIATED; } headers = parse->streamheader; @@ -343,7 +342,8 @@ gst_kate_parse_queue_buffer (GstKateParse * parse, GstBuffer * buf) if (granpos >= 0) { ret = gst_kate_parse_drain_queue (parse, granpos); } else { - GST_WARNING_OBJECT (parse, "granulepos < 0 (%lld)", granpos); + GST_ELEMENT_ERROR (parse, STREAM, DECODE, (NULL), + ("Bad granulepos %" G_GINT64_FORMAT, granpos)); ret = GST_FLOW_ERROR; } #endif diff --git a/ext/kate/gstkatespu.c b/ext/kate/gstkatespu.c index f05ae421..760662c4 100644 --- a/ext/kate/gstkatespu.c +++ b/ext/kate/gstkatespu.c @@ -253,7 +253,8 @@ gst_kate_spu_crop_bitmap (GstKateEnc * ke, kate_bitmap * kb, guint16 * dx, kb->height = h; } -#define CHECK(x) do { guint16 _ = (x); if (G_UNLIKELY((_) > sz)) { GST_WARNING_OBJECT (ke, "SPU overflow"); return GST_FLOW_ERROR; } } while (0) +/* FIXME: fix macros */ +#define CHECK(x) do { guint16 _ = (x); if (G_UNLIKELY((_) > sz)) { GST_ELEMENT_ERROR (ke, STREAM, ENCODE, ("Attempt to read outide buffer"), (NULL)); return GST_FLOW_ERROR; } } while (0) #define ADVANCE(x) do { guint16 _ = (x); ptr += (_); sz -= (_); } while (0) #define IGNORE(x) do { guint16 __ = (x); CHECK (__); ADVANCE (__); } while (0) @@ -267,8 +268,9 @@ gst_kate_spu_decode_command_sequence (GstKateEnc * ke, GstBuffer * buf, guint16 sz; if (command_sequence_offset >= GST_BUFFER_SIZE (buf)) { - GST_WARNING_OBJECT (ke, "Command sequence offset %u is out of range %u", - command_sequence_offset, GST_BUFFER_SIZE (buf)); + GST_ELEMENT_ERROR (ke, STREAM, DECODE, (NULL), + ("Command sequence offset %u is out of range %u", + command_sequence_offset, GST_BUFFER_SIZE (buf))); return GST_FLOW_ERROR; } @@ -348,10 +350,12 @@ gst_kate_spu_decode_command_sequence (GstKateEnc * ke, GstBuffer * buf, } break; default: - GST_WARNING_OBJECT (ke, "invalid SPU command: %u", cmd); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Invalid SPU command: %u", cmd)); return GST_FLOW_ERROR; } } + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), ("Error parsing SPU")); return GST_FLOW_ERROR; } @@ -395,9 +399,10 @@ gst_kate_spu_create_spu_palette (GstKateEnc * ke, kate_palette * kp) kate_palette_init (kp); kp->ncolors = 4; kp->colors = (kate_color *) g_malloc (kp->ncolors * sizeof (kate_color)); - if (G_UNLIKELY (!kp->colors)) + if (G_UNLIKELY (!kp->colors)) { + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), ("Out of memory")); return GST_FLOW_ERROR; - + } #if 1 for (n = 0; n < kp->ncolors; ++n) { int idx = ke->spu_colormap[n]; @@ -478,7 +483,8 @@ gst_kate_spu_decode_spu (GstKateEnc * ke, GstBuffer * buf, kate_region * kr, if (G_UNLIKELY (ke->spu_right - ke->spu_left < 0 || ke->spu_bottom - ke->spu_top < 0 || ke->spu_pix_data[0] == 0 || ke->spu_pix_data[1] == 0)) { - GST_WARNING_OBJECT (ke, "SPU area is empty, nothing to encode"); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("SPU area is empty, nothing to encode")); return GST_FLOW_ERROR; } @@ -495,7 +501,8 @@ gst_kate_spu_decode_spu (GstKateEnc * ke, GstBuffer * buf, kate_region * kr, kb->type = kate_bitmap_type_paletted; kb->pixels = (unsigned char *) g_malloc (kb->width * kb->height); if (G_UNLIKELY (!kb->pixels)) { - GST_WARNING_OBJECT (ke, "Failed to allocate memory for pixel data"); + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Failed to allocate memory for pixel data")); return GST_FLOW_ERROR; } diff --git a/ext/kate/gstkatetag.c b/ext/kate/gstkatetag.c index f5734c12..ac4e925f 100644 --- a/ext/kate/gstkatetag.c +++ b/ext/kate/gstkatetag.c @@ -63,8 +63,6 @@ * */ -/* FIXME: post appropriate GST_ELEMENT_ERROR when returning FLOW_ERROR */ - #ifdef HAVE_CONFIG_H # include "config.h" #endif diff --git a/ext/kate/gstkatetiger.c b/ext/kate/gstkatetiger.c index 3e72285a..433a3106 100644 --- a/ext/kate/gstkatetiger.c +++ b/ext/kate/gstkatetiger.c @@ -73,8 +73,6 @@ * */ -/* FIXME: post appropriate GST_ELEMENT_ERROR when returning FLOW_ERROR */ - #ifdef HAVE_CONFIG_H #include "config.h" #endif diff --git a/ext/kate/gstkateutil.c b/ext/kate/gstkateutil.c index ca245f99..f99247be 100644 --- a/ext/kate/gstkateutil.c +++ b/ext/kate/gstkateutil.c @@ -17,7 +17,6 @@ * Boston, MA 02111-1307, USA. */ -/* FIXME: post appropriate GST_ELEMENT_ERROR when returning FLOW_ERROR */ /* FIXME: shouldn't all this GstKateDecoderBase stuff really be a base class? */ #ifdef HAVE_CONFIG_H @@ -165,7 +164,8 @@ gst_kate_util_decoder_base_chain_kate_packet (GstKateDecoderBase * decoder, kate_packet_wrap (&kp, GST_BUFFER_SIZE (buf), GST_BUFFER_DATA (buf)); ret = kate_high_decode_packetin (&decoder->k, &kp, ev); if (G_UNLIKELY (ret < 0)) { - GST_WARNING_OBJECT (element, "kate_high_decode_packetin failed (%d)", ret); + GST_ELEMENT_ERROR (element, STREAM, DECODE, (NULL), + ("Failed to decode Kate packet: %d", ret)); return GST_FLOW_ERROR; } else if (G_UNLIKELY (ret > 0)) { GST_DEBUG_OBJECT (element, -- cgit v1.2.1 From a20d86f1f525f79779727972925283910dc4173d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Sat, 25 Jul 2009 12:19:07 +0100 Subject: kate: break up macros into multiple lines --- ext/kate/gstkatespu.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'ext/kate') diff --git a/ext/kate/gstkatespu.c b/ext/kate/gstkatespu.c index 760662c4..cc54cc4f 100644 --- a/ext/kate/gstkatespu.c +++ b/ext/kate/gstkatespu.c @@ -253,10 +253,21 @@ gst_kate_spu_crop_bitmap (GstKateEnc * ke, kate_bitmap * kb, guint16 * dx, kb->height = h; } -/* FIXME: fix macros */ -#define CHECK(x) do { guint16 _ = (x); if (G_UNLIKELY((_) > sz)) { GST_ELEMENT_ERROR (ke, STREAM, ENCODE, ("Attempt to read outide buffer"), (NULL)); return GST_FLOW_ERROR; } } while (0) -#define ADVANCE(x) do { guint16 _ = (x); ptr += (_); sz -= (_); } while (0) -#define IGNORE(x) do { guint16 __ = (x); CHECK (__); ADVANCE (__); } while (0) +#define CHECK(x) G_STMT_START { \ + guint16 _ = (x); \ + if (G_UNLIKELY((_) > sz)) { \ + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), ("Read outside buffer")); \ + return GST_FLOW_ERROR; \ + } \ + } G_STMT_END +#define ADVANCE(x) G_STMT_START { \ + guint16 _ = (x); ptr += (_); sz -= (_); \ + } G_STMT_END +#define IGNORE(x) G_STMT_START { \ + guint16 __ = (x); \ + CHECK (__); \ + ADVANCE (__); \ + } G_STMT_END static GstFlowReturn gst_kate_spu_decode_command_sequence (GstKateEnc * ke, GstBuffer * buf, -- cgit v1.2.1 From 24217ee31a5030f528e13a94233580e86d70baf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Sat, 8 Aug 2009 12:20:55 +0100 Subject: kate: some minor clean-ups Print flow return as string in log message; if we check the return value of gst_buffer_new_and_alloc() we should use the _try() function that might actually return NULL. Post error message when returning GST_FLOW_ERROR. Use portable GLib macros to print 64-bit integers. Don't use 0LL, that's also not portable (and unneeded here). --- ext/kate/gstkateenc.c | 19 +++++++++++-------- ext/kate/gstkateparse.c | 4 ++-- 2 files changed, 13 insertions(+), 10 deletions(-) (limited to 'ext/kate') diff --git a/ext/kate/gstkateenc.c b/ext/kate/gstkateenc.c index fd412d34..42333509 100644 --- a/ext/kate/gstkateenc.c +++ b/ext/kate/gstkateenc.c @@ -397,7 +397,7 @@ gst_kate_enc_create_buffer (GstKateEnc * ke, kate_packet * kp, { GstBuffer *buffer; - buffer = gst_buffer_new_and_alloc (kp->nbytes); + buffer = gst_buffer_try_new_and_alloc (kp->nbytes); if (G_UNLIKELY (!buffer)) { GST_WARNING_OBJECT (ke, "Failed to allocate buffer for %u bytes", kp->nbytes); @@ -425,7 +425,7 @@ gst_kate_enc_create_buffer (GstKateEnc * ke, kate_packet * kp, static GstFlowReturn gst_kate_enc_push_buffer (GstKateEnc * ke, GstBuffer * buffer) { - GstFlowReturn rflow; + GstFlowReturn flow; ke->last_timestamp = GST_BUFFER_TIMESTAMP (buffer); if (GST_BUFFER_TIMESTAMP (buffer) + GST_BUFFER_DURATION (buffer) > @@ -437,12 +437,12 @@ gst_kate_enc_push_buffer (GstKateEnc * ke, GstBuffer * buffer) /* Hack to flush each packet on its own page - taken off the CMML encoder element */ GST_BUFFER_DURATION (buffer) = G_MAXINT64; - rflow = gst_pad_push (ke->srcpad, buffer); - if (G_UNLIKELY (rflow != GST_FLOW_OK)) { - GST_ERROR_OBJECT (ke, "Failed to push buffer: %d", rflow); + flow = gst_pad_push (ke->srcpad, buffer); + if (G_UNLIKELY (flow != GST_FLOW_OK)) { + GST_WARNING_OBJECT (ke->srcpad, "push flow: %s", gst_flow_get_name (flow)); } - return rflow; + return flow; } static GstFlowReturn @@ -548,9 +548,12 @@ gst_kate_enc_send_headers (GstKateEnc * ke) kate_packet kp; int ret = kate_encode_headers (&ke->k, &ke->kc, &kp); if (ret == 0) { - GstBuffer *buffer = - gst_kate_enc_create_buffer (ke, &kp, 0ll, 0ll, 0ll, TRUE); + GstBuffer *buffer; + + buffer = gst_kate_enc_create_buffer (ke, &kp, 0, 0, 0, TRUE); if (!buffer) { + GST_ELEMENT_ERROR (ke, STREAM, ENCODE, (NULL), + ("Failed to create buffer, %u bytes", kp.nbytes)); rflow = GST_FLOW_ERROR; break; } diff --git a/ext/kate/gstkateparse.c b/ext/kate/gstkateparse.c index e9bcd5ae..e4a83477 100644 --- a/ext/kate/gstkateparse.c +++ b/ext/kate/gstkateparse.c @@ -242,7 +242,7 @@ static GstFlowReturn gst_kate_parse_push_buffer (GstKateParse * parse, GstBuffer * buf, gint64 granulepos) { - GST_LOG_OBJECT (parse, "granulepos %16llx", granulepos); + GST_LOG_OBJECT (parse, "granulepos %16" G_GINT64_MODIFIER "x", granulepos); if (granulepos < 0) { /* packets coming not from Ogg won't have a granpos in the offset end, so we have to synthesize one here - only problem is we don't know @@ -330,7 +330,7 @@ gst_kate_parse_queue_buffer (GstKateParse * parse, GstBuffer * buf) /* oggdemux stores the granule pos in the offset end */ granpos = GST_BUFFER_OFFSET_END (buf); - GST_LOG_OBJECT (parse, "granpos %16llx", granpos); + GST_LOG_OBJECT (parse, "granpos %16" G_GINT64_MODIFIER "x", granpos); g_queue_push_tail (parse->buffer_queue, buf); #if 1 -- cgit v1.2.1