[gstreamer-bugs] [Bug 337026] oggmux doesn't set EOS properly

[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=337026
 GStreamer | gst-plugins-base | Ver: HEAD CVS


James "Doc" Livingston changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #68765|0                           |1
        is obsolete|                            |




------- Comment #19 from James "Doc" Livingston  2006-08-07 11:44 UTC -------
Created an attachment (id=70380)
 --> (http://bugzilla.gnome.org/attachment.cgi?id=70380&action=view)
improved patch

(In reply to comment #18)
> 'has been removed'
> 
> Also, can you change the comments on buffer/next_buffer to be clearer about
> what they are, exactly? 
>
> Please don't add > 80 column lines.

Done.


> Not clear why you're using g_atomic_ things. If that's neccesary, then probably
> it's not sufficient - you should be taking the object lock, perhaps?

Changed back into normal artithmetic, as there shouldn't be any threading
issues.


> You move removing the pad from collectpads from the release_pad method to the
> main collectpads collected function. Why? Is this neccesary? Seems strange...

It was a leftover from an earlier attempt to fix a queue-flushing issue. It's
now gone.


> Recursively calling your collectpads collected function looks... ugly, and it's
> really hard to convince myself that it can't get things wrong. It's just to
> clear out the queues at EOS, right? Perhaps you can factor that out into
> another function, and just call that.

I've factored all the actual work out of the collectpads callback into a new
function _process_best_pad, which gets called once normally from the callback,
and in a loop when we're flushing the queues.

I've re-looked over it after writing it the other day, and I'm fairly certain I
handle things like flow-return properly.


> Otherwise, the patch itself looks pretty good. I haven't checked your tests,
> though.

If you have any suggestions for more tests I should add, let me know.


-- 
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.