[gstreamer-bugs] [Bug 332031] [PATCH] avimux ported to 0.10

[GStreamer (bugzilla.gnome.org)]

Do not reply to this via email (we are currently unable to handle email
responses and they get discarded).  You can add comments to this bug at
http://bugzilla.gnome.org/show_bug.cgi?id=332031
 GStreamer | gst-plugins-good | Ver: 0.10.2


Tim-Philipp Müller changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #59854|none                        |commented-on
               Flag|                            |
  Attachment #63818|none                        |commented-on
               Flag|                            |
  Attachment #63994|none                        |commented-on
               Flag|                            |
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1




------- Comment #9 from Tim-Philipp Müller  2006-04-27 09:26 UTC -------
Hi Mark, great work!

Made the patch apply against CVS HEAD and compile with -Wall -Werror and did
some clean-ups here and there (most of which I forgot or which were minor
things).

Here are some small things I ran into (fixed in my local copy):

- GST_PAD_PARENT does not return a reference, no gst_object_unref'ing
  needed, whereas gst_pad_get_parent() does return a reference and one
  needs to gst_object_unref() when done (these might not have been part
  of your patch though, but part of a mass update in CVS, didn't bother
  to check)

- gst_pad_get_name() returns a newly alloc'ed string which must be freed
  by g_free(), so it shouldn't be used in debug statements. GST_PAD_NAME
  or GST_DEBUG_PAD_NAME (with %s:%s string formatter) are better for
  debug statements (this is different from 0.8 where these didn't leak).

- one should generally set correct caps on all outgoing buffers
  (but absolutely must do that for the first outgoing buffer).

- gst_avimux_stop_file():
  GST_ELEMENT_ERROR should be used only for fatal things. If it
  is fatal, then you should also return GST_FLOW_ERROR, otherwise
  GST_ELEMENT_WARNING should do the job (otherwise the application
  won't know that the error message you send is not fatal).

- gst_avimux_handle_event():
  don't leak taglist extracted from tag event.

- commented out audio_pad_eos and video_pad_eos members - are they
  actually needed/used anywhere?

- gst_avimux_strip_buffer():
  can't assume it's safe to modify metadata on buffers like this (even
  though it probably is, but still). Must use
  gst_buffer_make_metadata_writable().

- the assumption is that the incoming streams are perfect streams, right?
  (ie. no gaps)

- gst_avimux_do_one_buffer():
  are avimux->video_buffer_queue and avimux->audio_buffer_queue needed
  for anything? Looks to me like they could just as well be replaced
  by local variables.

- gst_avimux_do_one_buffer():
  flow return on EOS should be GST_FLOW_UNEXPECTED not GST_FLOW_WRONG_STATE.

- gst_avimux_write_index():
  needs to return GST_FLOW_OK at the end

Still tracking down something related to the test case, will post updated patch
as soon as that's fixed.


-- 
Configure bugmail: http://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.