After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 656775 - oggmux: various cleanups
oggmux: various cleanups
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal minor
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-08-17 20:39 UTC by Vincent Penquerc'h
Modified: 2011-08-25 06:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
oggmux: headers should always have granpos 0 (1.43 KB, patch)
2011-08-17 20:40 UTC, Vincent Penquerc'h
committed Details | Review
oggmux: factor the header packet creation code (2.21 KB, patch)
2011-08-17 20:40 UTC, Vincent Penquerc'h
committed Details | Review
oggmux: use GST_WARNING_OBJECT for warnings, not g_warning (1.47 KB, patch)
2011-08-17 20:40 UTC, Vincent Penquerc'h
rejected Details | Review
ogg: rationalize serialno type to guint32 (12.45 KB, patch)
2011-08-17 20:40 UTC, Vincent Penquerc'h
committed Details | Review
oggmux: use oggstream to decide which BOS packets to place first (2.24 KB, patch)
2011-08-17 20:40 UTC, Vincent Penquerc'h
committed Details | Review
ogg: move the "always flush page" to oggstream (3.44 KB, patch)
2011-08-17 20:40 UTC, Vincent Penquerc'h
committed Details | Review
oggstream: new convenience function to get a stream's media type (1.81 KB, patch)
2011-08-17 20:41 UTC, Vincent Penquerc'h
committed Details | Review
oggstream: vorbis has a preroll of 2 (805 bytes, patch)
2011-08-18 08:31 UTC, Vincent Penquerc'h
committed Details | Review
ogg: get the operator precedence right, even if only a doc (1.04 KB, patch)
2011-08-18 08:38 UTC, Vincent Penquerc'h
committed Details | Review
ogg: use memory slices where appropriate (2.74 KB, patch)
2011-08-18 09:06 UTC, Vincent Penquerc'h
committed Details | Review
ogg: do not use 32 bit modifiers to print serial numbers (9.34 KB, patch)
2011-08-18 14:46 UTC, Vincent Penquerc'h
none Details | Review
ogg: do not use 32 bit modifiers to print serial numbers (9.34 KB, patch)
2011-08-18 15:05 UTC, Vincent Penquerc'h
none Details | Review
ogg: do not use 32 bit modifiers to print serial numbers (8.83 KB, patch)
2011-08-18 15:21 UTC, Vincent Penquerc'h
committed Details | Review

Description Vincent Penquerc'h 2011-08-17 20:39:31 UTC
soon in a repo near you.
Comment 1 Vincent Penquerc'h 2011-08-17 20:40:17 UTC
Created attachment 194078 [details] [review]
oggmux: headers should always have granpos 0
Comment 2 Vincent Penquerc'h 2011-08-17 20:40:25 UTC
Created attachment 194079 [details] [review]
oggmux: factor the header packet creation code
Comment 3 Vincent Penquerc'h 2011-08-17 20:40:32 UTC
Created attachment 194080 [details] [review]
oggmux: use GST_WARNING_OBJECT for warnings, not g_warning
Comment 4 Vincent Penquerc'h 2011-08-17 20:40:41 UTC
Created attachment 194081 [details] [review]
ogg: rationalize serialno type to guint32

It is a 32 bit unsigned number.
Sure, the libogg API uses a long, but that's an unfortunate oversight.
Comment 5 Vincent Penquerc'h 2011-08-17 20:40:49 UTC
Created attachment 194082 [details] [review]
oggmux: use oggstream to decide which BOS packets to place first

Ogg recommends video BOS packets to be first.
Use the "is_video" flag in oggstream to select those, rather than
check for known mime types.
Comment 6 Vincent Penquerc'h 2011-08-17 20:40:58 UTC
Created attachment 194083 [details] [review]
ogg: move the "always flush page" to oggstream

It avoids checking for specific media types in the muxer.
Comment 7 Vincent Penquerc'h 2011-08-17 20:41:08 UTC
Created attachment 194084 [details] [review]
oggstream: new convenience function to get a stream's media type

This will make logging a lot clearer, both in code and in output.
Comment 8 Vincent Penquerc'h 2011-08-18 08:31:26 UTC
Created attachment 194103 [details] [review]
oggstream: vorbis has a preroll of 2
Comment 9 Vincent Penquerc'h 2011-08-18 08:38:14 UTC
Created attachment 194106 [details] [review]
ogg: get the operator precedence right, even if only a doc
Comment 10 Vincent Penquerc'h 2011-08-18 09:06:14 UTC
Created attachment 194111 [details] [review]
ogg: use memory slices where appropriate

While there, avoid zeroing newly allocated memory where unnecessary
Comment 11 Sebastian Dröge (slomo) 2011-08-18 09:14:53 UTC
Comment on attachment 194080 [details] [review]
oggmux: use GST_WARNING_OBJECT for warnings, not g_warning

These are programming errors and they should be reported via g_warning(), same as all the g_return_val_if_fail(), etc
Comment 12 Sebastian Dröge (slomo) 2011-08-18 09:19:29 UTC
Pushed them all except the g_warning() patch, which is incorrect.
Comment 13 Vincent Penquerc'h 2011-08-18 14:46:01 UTC
Created attachment 194137 [details] [review]
ogg: do not use 32 bit modifiers to print serial numbers

If ints are 64 bits, 32 bits should get promoted in varargs anyway,
and we don't care about 16 bit ints.
This makes the code a lot more readable, and still gets us nice
hexadecimal 32 bit serialnos.
Comment 14 Vincent Penquerc'h 2011-08-18 15:05:47 UTC
Created attachment 194139 [details] [review]
ogg: do not use 32 bit modifiers to print serial numbers

If ints are 64 bits, 32 bits should get promoted in varargs anyway,
and we don't care about 16 bit ints.
This makes the code a lot more readable, and still gets us nice
hexadecimal 32 bit serialnos.
Comment 15 Vincent Penquerc'h 2011-08-18 15:21:57 UTC
Created attachment 194140 [details] [review]
ogg: do not use 32 bit modifiers to print serial numbers

If ints are 64 bits, 32 bits should get promoted in varargs anyway,
and we don't care about 16 bit ints.
This makes the code a lot more readable, and still gets us nice
hexadecimal 32 bit serialnos.

Trying again on a clean branch, my ogg branch is getting too
crowded and patches are starting to conflict with each others, sorry.
Comment 16 Vincent Penquerc'h 2011-08-18 16:13:03 UTC
Thanks for pushing.
You marked https://bugzilla.gnome.org/attachment.cgi?id=194111 as committed but have not pushed it though.
Cheers
Comment 17 Tim-Philipp Müller 2011-08-18 19:20:34 UTC
commit b7bb1e56336b9d3167213e0f48fa0a25ea4b0ad4
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Thu Aug 18 16:20:57 2011 +0100

    ogg: do not use 32 bit modifiers to print serial numbers
    
    If ints are 64 bits, 32 bits should get promoted in varargs anyway,
    and we don't care about 16 bit ints.
    This makes the code a lot more readable, and still gets us nice
    hexadecimal 32 bit serialnos.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=656775
Comment 18 Vincent Penquerc'h 2011-08-24 16:46:53 UTC
Reopening so we don't forget one patch is still pending.
Comment 19 Sebastian Dröge (slomo) 2011-08-25 06:27:32 UTC
commit 53c865624827f428247ea80f62db1c0ff12a6dd2
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Thu Aug 18 10:05:17 2011 +0100

    ogg: use memory slices where appropriate
    
    While there, avoid zeroing newly allocated memory where unnecessary
    
    https://bugzilla.gnome.org/show_bug.cgi?id=656775