GNOME Bugzilla – Bug 337409
[2.0] encoder elements should all use bits/sec as the unit for bitrate properties
Last modified: 2018-11-03 11:12:25 UTC
theoraenc for example uses kbit/sec it's an ABI change so we have to do it in 0.11
Should those bitrates in bit/s be with a base of 1000 or 1024 for kbit?
It should be whatever the library in question is actually using (from looking at code/description/...) if theora uses 1024 bits for a kbit, then we use that too, for example. Going to bits/sec everywhere makes that less ambiguous.
*** Bug 371314 has been marked as a duplicate of this bug. ***
Also, they should be the same type (either unsigned long or unsigned int). Currently, some use unsigned long, some unsigned int, some signed int.
Created attachment 167425 [details] [review] utility function to set bitrates (in bit/sec) Until we get to the bright world of 0.11, we're stuck with having to put element-special knowledge into out applications. I was going to write a function with that knowledge and put it into farsight, but I guess this could be useful to others and belongs in the gst core.
Created attachment 167426 [details] [review] updated utility function to set bitrate oops, previous one touch an unrelated line of whitespace
Created attachment 167427 [details] [review] make celtenc use bits/sec The only element in -bad that uses kbps is celtenc, lets fix it.
This seems awkward to me. How about an interface, or a new standardised property?
(In reply to comment #8) > This seems awkward to me. How about an interface, or a new standardised > property? You mean an interface with a interface property ;) If we ever add such interface, it should be a GstEncoder interface with more settings
Its not only encoders (I think there are some other elements like parsers that also have a bitrate property). I can write patches to add a "bitrate-in-bits" property or something.
A encoder baseclass would be nice, otherwise an interface. Olivier, why would one need to *set* a bitrate on a parser?
ah no sorry, the "bitrate" property on mp3parse is read-only.. so I guess its only for encoders
Comment on attachment 167427 [details] [review] make celtenc use bits/sec Committed the patch to celtenc, lets try to make sure all the new elements are guint+bits/sec commit 3cb4a7aefd679925fb46c773b09f4c964514839a Author: Olivier Crête <olivier.crete@collabora.co.uk> Date: Mon Aug 9 14:32:57 2010 +0200 celtenc: Change bitrate to bits/sec
Maybe a bit extreme, but is it worth using uint64_t for future proofing and allow huge bitrates (> 4 GB/s) ?
1.0 is almost there.. we should fix this before the API is frozen for another 5 years. Should we also standardise it to uint? Or maybe kbit/sec? Open to suggestions.
We seem to have dropped the ball on this one. Should I commit the utility function for the 1.x cycle ?
> We seem to have dropped the ball on this one. Should I commit the utility > function for the 1.x cycle ? I'm not so keen on these under the GstElement namespace. I think there should be a GstEncoder interface at some point, with API for this. Maybe mark as duplicate of bug #118142 ?
bug #118142 seems like 2.0 material also.. Maybe I should move this under pbutils or something ? Although it doesn't really fit under any of the existing libraries except the core.
Why is a new encoder interface 2.0 material? It's maybe not ready-in-2-minutes material, but 2.0? I would prefer we work on the encoder interface idea. It doesn't have to be complete from the start either API-wise, I think. (Or does it?) Another thing: do we have a list of elements affected? Is it mostly kbps vs bps or also bytes/sec vs. bits/sec? In many cases we can probably change it to bps and still accept the old values if it's a range that doesn't really make sense otherwise?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/2.