This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 485245 - Review Request: celt051 - An audio codec for use in low-delay speech and audio communication
Review Request: celt051 - An audio codec for use in low-delay speech and audi...
Status: CLOSED NEXTRELEASE
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: Package Review (Show other bugs)
5.4
All Linux
medium Severity medium
: rc
: ---
Assigned To: Richard W.M. Jones
:
Depends On:
Blocks: 188273 488571
  Show dependency treegraph
 
Reported: 2009-02-12 11:00 EST by Monty
Modified: 2016-04-26 10:46 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-23 22:26:27 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Monty 2009-02-12 11:00:54 EST
Spec URL: http://file.rdu.redhat.com/~cmontgom/celt.spec
SRPM URL: http://file.rdu.redhat.com/~cmontgom/celt-0.5.1-2.el5.src.rpm
Description: 
CELT (Constrained Energy Lapped Transform) is an ultra-low delay audio 
codec designed for realtime transmission of high quality speech and audio. 
This is meant to close the gap between traditional speech codecs 
(such as Speex) and traditional audio codecs (such as Vorbis).
Comment 1 Ray Strode [halfline] 2009-02-12 13:18:11 EST
So this has already gone through two review requests.  See,

bug 478941
and
bug 479750

It's good to go as far as I'm concerned.  My only issue is that this package is already in EPEL, so I'm not really sure what the procedure should be.  I guess we'll need to talk to Peter Robinson when this lands, so he's aware of the situation.
Comment 2 Richard W.M. Jones 2009-03-05 05:43:02 EST
Taking for review.

I did the Fedora review (bug 478941) so I'm going to base
my review on any differences between this package and the
one in Fedora.
Comment 3 Richard W.M. Jones 2009-03-05 05:52:27 EST
The difference is simply the version number (0.5.1 here
versus 0.5.2 in Fedora).  The difference in those upstream
versions is also not a great deal.

So ---> APPROVED <---
Comment 4 Richard W.M. Jones 2009-03-05 05:54:32 EST
(In reply to comment #1)
> It's good to go as far as I'm concerned.  My only issue is that this package is
> already in EPEL, so I'm not really sure what the procedure should be.  I guess
> we'll need to talk to Peter Robinson when this lands, so he's aware of the
> situation.

I emailed Peter about this.
Comment 5 Monty 2009-03-05 13:27:23 EST
Woah, wait:

0.5.1 and 0.5.2 are not bitstream compatible.  We're deploying CELT-0.5.1 as a targetted/embedded application and are doing so aware of this limitation.  0.5.2 does *not* obsolete or replace 0.5.1. They are not interchangable codecs. 0.5.1 MUST NOT be packaged as if 0.5.2 is its upgrade path.  Any user who is 'seamlessly' upgraded to 0.5.2 from 0.5.1 will be very unhappy.

Has Fedora already screwed this up?
Comment 6 Richard W.M. Jones 2009-03-05 13:34:41 EST
Ahem, they certainly don't mention that very prominently
in the docs.  Yes, we probably screwed this up in Fedora,
but to be honest 0.5.1 was only shipped for about a day so
it wasn't too much of a screw-up.

I will alert Peter anyway.
Comment 7 Gregory Maxwell 2009-03-05 13:41:45 EST
Richard, Can you recommend where we should best place the "BLINKING YELLOW LIGHT OF WARNING".  It's mentioned in a few places (readme, download page), but I can certainly make it more obvious.
Comment 8 Richard W.M. Jones 2009-03-05 13:54:08 EST
The warning I'm supposed to see is the text buried in the
block of text at the top of the page that says
"Neither the API/ABI, nor the bit-stream are stable"?

No one reads text on webpages, especially when they are
goal-seeking, as they would be on a download page, but
in general they don't read the text anyway.  So it really
would need to be 24 pt high blinking yellow text of doom
for anyone to pay it any notice ..
Comment 9 Monty 2009-03-05 14:02:00 EST
Actually, it's [at this temporary moment] a large problem as all the RH business products are shipping Spice clients with 0.5.1 and will be approximately forever (thus 'embedded application').  This was the whole point to packaging Celt at this time before bitstream freeze.  Fedora Spice will never work with a spice-enabled KVM until 0.5.1 is made available in some way where it's understood that 0.5.>1 is not the upgrade path. 

Even worse, the differing libraries make no attempt to detect incompatability-- the 'upgraded' libcelt will simply blow out the hapless users' ears without complaint.  The is the part of 'warning: prerelease, please pay attention' that meant 'warning: prerelease, please pay attention'.  

I'm sorry that the multiple upstream disclaimers stating that 0.5.2 was not compatible with 0.5.1 were completely ignored, though I agree this is an abuse of a subminor bump.  Oddly, I don't think I got any notification of the Fedora package changes, else I'd have piped up sooner.
Comment 10 Monty 2009-03-05 14:10:18 EST
(In reply to comment #8)

> No one reads text on webpages, especially when they are
> goal-seeking, as they would be on a download page, but
> in general they don't read the text anyway. 

I agree with you and that makes me sad. "Competent packaging is hard, let's go shopping."

gah, I wonder if the right thing now is to abandon 0.5.1 completely, move everyone forward forcibly, and resolve to not make the mistake again (yes, I have some confusion to end on the Xiph.Org end as well).
Comment 11 Peter Robinson 2009-03-05 14:25:01 EST
well 0.5.1 was never linked against anything in Fedora but I presume this means that in the case of ekiga if someone with an ekiga using celt 0.5.1 calls someone with ekiga using celt 0.5.2 this isn't going to work?
Comment 12 Gregory Maxwell 2009-03-05 14:39:02 EST
It's not 24pt high but I've added some additional warning text in bold to the celt-codec.org page, to the download page, and to the release notes. Obviously this doesn't fix the pre-existing problem, but hopefully it will reduce it in the future.

Regarding ekiga, thats correct. Ekiga is also not yet compatible with the (still being written) CELT RTP payload. (Ekiga did its own thing with CELT without consulting CELT development, which is fine, but what they did isn't something we can standardize on so it's not going to be interoperable until it is updated at some point in the future).
Comment 13 Monty 2009-03-05 14:40:35 EST
Correct.

RHEL has a few products about to be released that link to 0.5.1.  The Spice remote desktop protocol currently predicates CELT-0.5.1 as part of the protocol itself and that's one source of the dependency.
Comment 14 Peter Robinson 2009-03-05 15:06:02 EST
Is it worthwhile adding an epoch to the Fedora package and rolling it back to 0.5.1?
Comment 15 Monty 2009-03-05 16:42:46 EST
Yes, please. If this can be done without any unforseen procedural blockers, rolling back Fedora is what I'd prefer.  If that goes badly or not at all, there are other possibilities.

FYI:

CELT is not at full release or bitstream freeze because JM et al think that there may yet be significant improvements left to make and it is a relatively new codec.  That said, 0.5.1 has already proven itself stable, and no other free codec fills this vital niche.  We need it. For that reason, I opted (wearing both my Xiph and RedHat hats) to package and release this specific version as a stable preview, and commit to maintaining it on an upstream branch.

There was no plan to 'update' or offer a newer package until we knew more about how the CODEC was maturing, possibly not until a 1.0 bitstream freeze later this year.
Comment 16 Richard W.M. Jones 2009-03-05 17:32:30 EST
(In reply to comment #15)
> CELT is not at full release or bitstream freeze because JM et al think that
> there may yet be significant improvements left to make and it is a relatively
> new codec.  That said, 0.5.1 has already proven itself stable, and no other
> free codec fills this vital niche.  We need it. For that reason, I opted
> (wearing both my Xiph and RedHat hats) to package and release this specific
> version as a stable preview, and commit to maintaining it on an upstream
> branch.
> 
> There was no plan to 'update' or offer a newer package until we knew more about
> how the CODEC was maturing, possibly not until a 1.0 bitstream freeze later
> this year.  

What's the plan going forward then?  We could add an explicit
RPM provides, like:

  Provides: celt-codec = 0.5.1

which would allow other packages to depend on a specific
"variant".

However that won't work if any of the following happen: (a) two
versions on different machines talk to each other, (b) files are
saved in celt format and must be loaded in future.  I assume from
the ekiga connection that at least (a) is likely to occur?
Comment 17 Peter Robinson 2009-03-05 17:49:15 EST
> However that won't work if any of the following happen: (a) two
> versions on different machines talk to each other, (b) files are
> saved in celt format and must be loaded in future.  I assume from
> the ekiga connection that at least (a) is likely to occur?  

WRT to ekiga, I have a massive urge to just remove the celt support from ekiga until it stabilises some what. Reasons include:
- Not usable for anything except opal to opal (ekiga being the only opensource one) client using the same version of celt
- only supported in the upcoming ekiga 3.2 release which is only in rawhide
- ekiga in rawhide only got the support for celt yesterday

So its not like it currently has widespread use.

I think it will keep it more simple and it can then be added back in when the codec RTP and algorithm has stabilised.

I noticed there is also a gstreamer plugin but its not in fedora yet as its still in gst-plugins-bad
Comment 18 Juan Quintela 2009-03-17 18:55:16 EDT
Shouldn't we split in a lib package the libraries?

%{_bindir}/celtenc
%{_bindir}/celtdec
%{_libdir}/libcelt.so.0
%{_libdir}/libcelt.so.0.0.0

(yes, I have read the previous discussion, and still ask that :)
Comment 19 Richard W.M. Jones 2009-03-18 05:05:46 EDT
(In reply to comment #18)
> Shouldn't we split in a lib package the libraries?
> 
> %{_bindir}/celtenc
> %{_bindir}/celtdec
> %{_libdir}/libcelt.so.0
> %{_libdir}/libcelt.so.0.0.0
> 
> (yes, I have read the previous discussion, and still ask that :)  

Whatever we do, it should be discussed & done in Rawhide first.
Comment 20 Monty 2009-03-18 11:25:45 EDT
I had originally done it that way in my package but that was dropped in favor of rjones's approach, and I had assumed that was on purpose.
Comment 21 Richard W.M. Jones 2009-03-18 11:38:52 EDT
I wasn't involved in making this package, I just reviewed it.
Comment 22 Monty 2009-03-18 12:54:33 EDT
Argh, sorry rjones, this is a braino I keep making.  I meant probinson.
Comment 23 Peter Robinson 2009-03-18 20:20:34 EDT
I've got no probs splitting them out. Do you want them done alone the same lines as the speex package? Its also been on my list to roll it back to 0.5.1 with an epoch.
Comment 24 Richard W.M. Jones 2009-03-19 08:35:08 EDT
I think it's better to supply a compat package.

Fedora policy is that we should keep up with upstream:
http://fedoraproject.org/wiki/Staying_close_to_upstream_projects

I've never made a compat package, and I can't even find any
guideline on how it should be done.  Closest I can find is:
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Multiple_packages_with_the_same_base_name
Comment 25 Richard W.M. Jones 2009-03-19 13:57:31 EDT
This is the opinion of the Fedora packaging gurus:
https://www.redhat.com/archives/fedora-packaging/2009-March/msg00047.html
Comment 26 Monty 2009-03-25 12:59:48 EDT
So an explicitly versioned 'compat' package ala celt-0.5.1-version is the 'agreed' way to go? Sounds just fine to me.  I assume we're talking about about RHEL/RHEV here (and it's my action item) and pbrobinson will worry about Fedora?

Given the package naming change, what administratively needs to happen aside from altering the spec/etc?  Does the review process start afresh, or can this chain be morphed?
Comment 27 Richard W.M. Jones 2009-03-25 13:22:58 EDT
Yes, seems to be the best way to go.

From what I've read it looks like it should be named:
 celt051
 compat-celt051

Myself or pbrobinson can worry about Fedora.

I'm fairly sure it is pointless to go through package
review all over again.  I've reviewed both 0.5.1 and 0.5.2
now, and they are both perfectly fine.
Comment 28 Monty 2009-03-25 13:58:57 EDT
Is the suggestion to name it celt051 in RHEL and compat-celt051 in fedora?  Or to choose one to be used in both?  (I personally feel relatively strongly that celt051 is a better choice and compat-celt051, unless there's a specific reason celt051 shouldn't be used)
Comment 29 Richard W.M. Jones 2009-03-25 14:06:21 EDT
Yes, choose just one.  celt051 is fine.
Comment 30 Monty 2009-03-26 14:04:13 EDT
done.
Comment 31 Perry Myers 2009-03-27 20:21:36 EDT
Changing subject of BZ to use the new package name 'celt051'

Note You need to log in before you can comment on or make changes to this bug.