Bug 485245

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.

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.

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

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

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?

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.

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.

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

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.

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

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?

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

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.

Is it worthwhile adding an epoch to the Fedora package and rolling it back to 0.5.1?

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.

(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?

> 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

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 :)

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

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.

I wasn't involved in making this package, I just reviewed it.

Argh, sorry rjones, this is a braino I keep making.  I meant probinson.

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.

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

This is the opinion of the Fedora packaging gurus:
https://www.redhat.com/archives/fedora-packaging/2009-March/msg00047.html

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?

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.

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)

Yes, choose just one.  celt051 is fine.

done.

Changing subject of BZ to use the new package name 'celt051'