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 337409 - [2.0] encoder elements should all use bits/sec as the unit for bitrate properties
[2.0] encoder elements should all use bits/sec as the unit for bitrate proper...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: 2.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 371314 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-04-05 17:27 UTC by Thomas Vander Stichele
Modified: 2018-11-03 11:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
utility function to set bitrates (in bit/sec) (3.96 KB, patch)
2010-08-09 13:29 UTC, Olivier Crête
none Details | Review
updated utility function to set bitrate (3.66 KB, patch)
2010-08-09 13:33 UTC, Olivier Crête
reviewed Details | Review
make celtenc use bits/sec (3.11 KB, patch)
2010-08-09 13:33 UTC, Olivier Crête
committed Details | Review

Description Thomas Vander Stichele 2006-04-05 17:27:17 UTC
theoraenc for example uses kbit/sec

it's an ABI change so we have to do it in 0.11
Comment 1 Tim-Philipp Müller 2006-06-13 19:05:58 UTC
Should those bitrates in bit/s be with a base of 1000 or 1024 for kbit?
Comment 2 Thomas Vander Stichele 2006-07-02 09:39:46 UTC
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.
Comment 3 David Schleef 2006-11-06 08:53:53 UTC
*** Bug 371314 has been marked as a duplicate of this bug. ***
Comment 4 Olivier Crête 2010-08-04 11:40:41 UTC
Also, they should be the same type (either unsigned long or unsigned int). Currently, some use unsigned long, some unsigned int, some signed int.
Comment 5 Olivier Crête 2010-08-09 13:29:49 UTC
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.
Comment 6 Olivier Crête 2010-08-09 13:33:08 UTC
Created attachment 167426 [details] [review]
updated utility function to set bitrate

oops, previous one touch an unrelated line of whitespace
Comment 7 Olivier Crête 2010-08-09 13:33:55 UTC
Created attachment 167427 [details] [review]
make celtenc use bits/sec

The only element in -bad that uses kbps is celtenc, lets fix it.
Comment 8 Tim-Philipp Müller 2010-08-09 17:41:26 UTC
This seems awkward to me. How about an interface, or a new standardised property?
Comment 9 Sebastian Dröge (slomo) 2010-08-09 18:01:56 UTC
(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
Comment 10 Olivier Crête 2010-08-09 21:21:17 UTC
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.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2010-08-12 12:01:31 UTC
A encoder baseclass would be nice, otherwise an interface. Olivier, why would one need to *set* a bitrate on a parser?
Comment 12 Olivier Crête 2010-08-12 12:12:20 UTC
ah no sorry, the "bitrate" property on mp3parse is read-only.. so I guess its only for encoders
Comment 13 Olivier Crête 2010-09-13 21:33:08 UTC
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
Comment 14 Vincent Penquerc'h 2012-01-09 12:52:46 UTC
Maybe a bit extreme, but is it worth using uint64_t for future proofing and allow huge bitrates (> 4 GB/s) ?
Comment 15 Olivier Crête 2012-04-12 23:36:07 UTC
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.
Comment 16 Olivier Crête 2013-01-21 19:25:20 UTC
We seem to have dropped the ball on this one. Should I commit the utility function for the 1.x cycle ?
Comment 17 Tim-Philipp Müller 2013-01-21 21:44:42 UTC
> 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 ?
Comment 18 Olivier Crête 2013-01-22 06:31:34 UTC
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.
Comment 19 Tim-Philipp Müller 2013-01-22 09:05:00 UTC
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?
Comment 20 GStreamer system administrator 2018-11-03 11:12:25 UTC
-- 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.