GNOME Bugzilla – Bug 719849
[PLUGIN-MOVE] move mpg123 to -ugly
Last modified: 2016-02-16 11:06:03 UTC
I think it is time to move the mpg123 plugin to -ugly. Looking at the moving-plugins document, I do not see any missing points.
Let's try to get it done for 1.4.0
Needs reviews btw, otherwise it seems to be done
Partial review: - gst_pad_get_allowed_caps() can return NULL if thepad is not linked, check for null value, example: gst-launch-1.0 filesrc location=file.mp3 ! mpegaudioparse ! mpg123audiodec - In more than one place, it return GST_FLOW_ERROR without doing GST_ELEMENT_ERROR(), ie without posting an error message - parsed=true shouldn't be in the static caps, otherwise it fails strangely without a parser. May want to check if parsed=true in set_format too.
Created attachment 266869 [details] [review] Error reporting fixes and checks to correct reported problems
The previously attached patch fixes the GST_FLOW_ERROR/GST_ELEMENT_ERROR() and the allowed_caps issues. The one with the "parsed" caps is fixed in git already.
Review of attachment 266869 [details] [review]: ::: ext/mpg123/gstmpg123audiodec.c @@ +339,3 @@ } else { + GST_ELEMENT_ERROR (mpg123_decoder, RESOURCE, READ, (NULL), + ("gst_memory_map() failed")); Please use GST_AUDIO_DECODER_ERROR(), it will only error out completely after a few consecutive errors then @@ +423,3 @@ char const *errmsg = mpg123_plain_strerror (errcode); + GST_ELEMENT_ERROR (mpg123_decoder, STREAM, DECODE, (NULL), + ("mpg123 decoding error: %s", errmsg)); And here
Created attachment 268081 [details] [review] Error reporting fixes and checks to correct reported problems, v2
Created attachment 268082 [details] [review] Error reporting fixes and checks to correct reported problems, v3 fixed wrong email address in the header
Also, since 1.22.4 libmpg123 supports "freeformat" mp3s again, so we should consider bumping its rank above mad and deprecate mad (which is also GPL and tends to be slow/thorough as well).
Too late for 1.6, shoudl we revisit after it is out?
Let's try to get this done for 1.8. Comments?
Can't we just wait until 2017? ;)
commit 08d8aefcdaaf89ecb6dd53ec1e4f95cd42d01664 Author: Tim-Philipp Müller <tim@centricular.com> Date: Tue Feb 16 10:44:33 2016 +0000 mpg123: move plugin from -bad to -ugly https://bugzilla.gnome.org/show_bug.cgi?id=719849 ---- commit 43bd45ba991ef3247957ca37cdcb52f4b8c0acb1 Author: Tim-Philipp Müller <tim@centricular.com> Date: Tue Feb 16 10:38:18 2016 +0000 mpg123: move from -bad to -ugly Hook up to build system, add to docs commit fadda9dba65a9de0fab0adc541d528f93384059c Author: Tim-Philipp Müller <tim@centricular.com> Date: Tue Feb 16 10:55:01 2016 +0000 tests: add test data for mpg123 plugin commit e3bb9b292823a3d7c823b5845786cdf84074f10b Merge: 7e2f2f9 93b15dd Author: Tim-Philipp Müller <tim@centricular.com> Date: Tue Feb 16 10:41:07 2016 +0000 Merge branch 'plugin-move-mpg123' Move mpg123 plugin from -bad to -ugly. https://bugzilla.gnome.org/show_bug.cgi?id=719849