[Bug 686595] [mpg123] misc improvements and fixes

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=686595
  GStreamer | gst-plugins-bad | 1.0.0

Tim-Philipp Müller <t.i.m> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |t.i.m at zen.co.uk
     Ever Confirmed|0                           |1

--- Comment #1 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2012-10-22 12:11:41 UTC ---
Thanks, but perhaps you could break things up in smaller pieces next time (not
20, but perhaps also not one megapatch).

Now, if I may nitpick a little:

 - set_format: perhaps "input_caps", but
    "upstream_caps" is not a term that's
    usually used in such a context.

 - for comments we prefer each line to
   have a '*'

 - an mpg123_open*() error is more a
   LIBRARY, INIT error than a
   RESOURCE, OPEN_READ (that's more
   for a device like a CD drive or so)

 - TRACE log level is for very frequently
    recurring things that add very little
    info in most cases (e.g. refcounting
    changes). Things in ::start/stop
    functions should be INFO or DEBUG

 -  the "nothing was decoded" msg
    should probably be DEBUG rather
    than TRACE too, for example
    (not recurring during normal
    operation); and perhaps have sth
    added that it's nor something to
    be alarmed about, like 'need more
    frames' or 'need more data' or so

 - if you allocate a buffer for num_decoded_bytes
   and you get a buffer back, I don't think
   you need to check after the map
   call that you really have as many bytes
   as you allocated... (neither less nor
   more).

 - in handle_frame, why do you get and
   map the GstMemory instead of the buffer?

 - if gst_audio_decoder_set_output_format()
   fails, you should probably just return
   FLOW_NOT_NEGOTIATED, and maybe
   log something to the debug log with
   GST_WARNING or so (and not post
   an error message)

 - not sure about the error stuff, ideally the
   decoder would just ignore anything that's
   not absolutely fatal and try to resync
   (and ignore upstream caps if in doubt..)

 - perhaps the test data for the unit test could
   be dumped into files in tests/files/ ? Makes
   the code easier to peruse..

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.